varlink_error_invalid_parameter(...) always returns EINVAL

varlink_error(...) expects a json object as the third parameter. Passing a string variant causes
parameter sanitization to fail, and it returns -EINVAL. Pass object variant instead.

src/core/execute.c

@@ -6080,9 +6080,11 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) {
         }
 
 #if HAVE_LIBBPF
-        if (exec_context_restrict_filesystems_set(c))
+        if (exec_context_restrict_filesystems_set(c)) {
-                SET_FOREACH(e, c->restrict_filesystems)
+                char *fs;
-                        fprintf(f, "%sRestrictFileSystems: %s\n", prefix, *e);
+                SET_FOREACH(fs, c->restrict_filesystems)
+                        fprintf(f, "%sRestrictFileSystems: %s\n", prefix, fs);
+        }
 #endif
 
         if (c->network_namespace_path)

src/libsystemd-network/sd-radv.c

@@ -578,8 +578,7 @@ int sd_radv_set_preference(sd_radv *ra, unsigned preference) {
 
 int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
         _cleanup_free_ char *addr_p = NULL;
-        sd_radv_prefix *cur;
+        sd_radv_prefix *cur, *found = NULL;
-        bool update = false;
         int r;
 
         assert_return(ra, -EINVAL);
@@ -604,7 +603,7 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
                         continue;
 
                 if (cur->opt.prefixlen == p->opt.prefixlen) {
-                        update = true;
+                        found = cur;
                         break;
                 }
 
@@ -615,15 +614,13 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
                                       strna(addr_p), strna(addr_cur));
         }
 
-        if (update) {
+        if (found) {
-                assert(cur);
-
                 /* p and cur may be equivalent. First increment the reference counter. */
                 sd_radv_prefix_ref(p);
 
                 /* Then, remove the old entry. */
-                LIST_REMOVE(prefix, ra->prefixes, cur);
+                LIST_REMOVE(prefix, ra->prefixes, found);
-                sd_radv_prefix_unref(cur);
+                sd_radv_prefix_unref(found);
 
                 /* Finally, add the new entry. */
                 LIST_APPEND(prefix, ra->prefixes, p);
@@ -659,15 +656,20 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
         return 0;
 }
 
-sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra,
+void sd_radv_remove_prefix(
-                                               const struct in6_addr *prefix,
+                sd_radv *ra,
-                                               unsigned char prefixlen) {
+                const struct in6_addr *prefix,
-        sd_radv_prefix *cur, *next;
+                unsigned char prefixlen) {
 
-        assert_return(ra, NULL);
+        sd_radv_prefix *cur;
-        assert_return(prefix, NULL);
 
-        LIST_FOREACH_SAFE(prefix, cur, next, ra->prefixes) {
+        if (!ra)
+                return;
+
+        if (!prefix)
+                return;
+
+        LIST_FOREACH(prefix, cur, ra->prefixes) {
                 if (prefixlen != cur->opt.prefixlen)
                         continue;
 
@@ -677,17 +679,13 @@ sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra,
                 LIST_REMOVE(prefix, ra->prefixes, cur);
                 ra->n_prefixes--;
                 sd_radv_prefix_unref(cur);
-
+                return;
-                break;
         }
-
-        return cur;
 }
 
 int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
         _cleanup_free_ char *addr_p = NULL;
-        sd_radv_route_prefix *cur;
+        sd_radv_route_prefix *cur, *found = NULL;
-        bool update = false;
         int r;
 
         assert_return(ra, -EINVAL);
@@ -708,7 +706,7 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
                         continue;
 
                 if (cur->opt.prefixlen == p->opt.prefixlen) {
-                        update = true;
+                        found = cur;
                         break;
                 }
 
@@ -719,15 +717,13 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
                                       strna(addr_p), strna(addr_cur));
         }
 
-        if (update) {
+        if (found) {
-                assert(cur);
-
                 /* p and cur may be equivalent. First increment the reference counter. */
                 sd_radv_route_prefix_ref(p);
 
                 /* Then, remove the old entry. */
-                LIST_REMOVE(prefix, ra->route_prefixes, cur);
+                LIST_REMOVE(prefix, ra->route_prefixes, found);
-                sd_radv_route_prefix_unref(cur);
+                sd_radv_route_prefix_unref(found);
 
                 /* Finally, add the new entry. */
                 LIST_APPEND(prefix, ra->route_prefixes, p);
@@ -762,8 +758,12 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
         return 0;
 }
 
-int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime,
+int sd_radv_set_rdnss(
-                               const struct in6_addr *dns, size_t n_dns) {
+                sd_radv *ra,
+                uint32_t lifetime,
+                const struct in6_addr *dns,
+                size_t n_dns) {
+
         _cleanup_free_ struct sd_radv_opt_dns *opt_rdnss = NULL;
         size_t len;
 
@@ -796,8 +796,11 @@ int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime,
         return 0;
 }
 
-int sd_radv_set_dnssl(sd_radv *ra, uint32_t lifetime,
+int sd_radv_set_dnssl(
-                               char **search_list) {
+                sd_radv *ra,
+                uint32_t lifetime,
+                char **search_list) {
+
         _cleanup_free_ struct sd_radv_opt_dns *opt_dnssl = NULL;
         size_t len = 0;
         char **s;
@@ -876,8 +879,11 @@ int sd_radv_prefix_new(sd_radv_prefix **ret) {
 
 DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_radv_prefix, sd_radv_prefix, mfree);
 
-int sd_radv_prefix_set_prefix(sd_radv_prefix *p, const struct in6_addr *in6_addr,
+int sd_radv_prefix_set_prefix(
-                                       unsigned char prefixlen) {
+                sd_radv_prefix *p,
+                const struct in6_addr *in6_addr,
+                unsigned char prefixlen) {
+
         assert_return(p, -EINVAL);
         assert_return(in6_addr, -EINVAL);
 
@@ -894,8 +900,11 @@ int sd_radv_prefix_set_prefix(sd_radv_prefix *p, const struct in6_addr *in6_addr
         return 0;
 }
 
-int sd_radv_prefix_get_prefix(sd_radv_prefix *p, struct in6_addr *ret_in6_addr,
+int sd_radv_prefix_get_prefix(
-                                       unsigned char *ret_prefixlen) {
+                sd_radv_prefix *p,
+                struct in6_addr *ret_in6_addr,
+                unsigned char *ret_prefixlen) {
+
         assert_return(p, -EINVAL);
         assert_return(ret_in6_addr, -EINVAL);
         assert_return(ret_prefixlen, -EINVAL);
@@ -914,8 +923,7 @@ int sd_radv_prefix_set_onlink(sd_radv_prefix *p, int onlink) {
         return 0;
 }
 
-int sd_radv_prefix_set_address_autoconfiguration(sd_radv_prefix *p,
+int sd_radv_prefix_set_address_autoconfiguration(sd_radv_prefix *p, int address_autoconfiguration) {
-                                                          int address_autoconfiguration) {
         assert_return(p, -EINVAL);
 
         SET_FLAG(p->opt.flags, ND_OPT_PI_FLAG_AUTO, address_autoconfiguration);
@@ -967,8 +975,11 @@ int sd_radv_route_prefix_new(sd_radv_route_prefix **ret) {
 
 DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_radv_route_prefix, sd_radv_route_prefix, mfree);
 
-int sd_radv_route_prefix_set_prefix(sd_radv_route_prefix *p, const struct in6_addr *in6_addr,
+int sd_radv_route_prefix_set_prefix(
-                                             unsigned char prefixlen) {
+                sd_radv_route_prefix *p,
+                const struct in6_addr *in6_addr,
+                unsigned char prefixlen) {
+
         assert_return(p, -EINVAL);
         assert_return(in6_addr, -EINVAL);
 

src/network/networkd-dhcp-prefix-delegation.c

@@ -187,7 +187,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) {
                                 continue;
 
                         if (link->radv)
-                                (void) sd_radv_remove_prefix(link->radv, &route->dst.in6, 64);
+                                sd_radv_remove_prefix(link->radv, &route->dst.in6, 64);
 
                         link_remove_dhcp_pd_subnet_prefix(link, &route->dst.in6);
 
@@ -212,7 +212,7 @@ int dhcp_pd_remove(Link *link, bool only_marked) {
                         in6_addr_mask(&prefix, 64);
 
                         if (link->radv)
-                                (void) sd_radv_remove_prefix(link->radv, &prefix, 64);
+                                sd_radv_remove_prefix(link->radv, &prefix, 64);
 
                         link_remove_dhcp_pd_subnet_prefix(link, &prefix);
 

src/partition/repart.c

@@ -1778,11 +1778,11 @@ static int context_load_partition_table(
                 }
 
                 sz = fdisk_partition_get_size(p);
-                assert_se(sz <= UINT64_MAX/secsz);
+                assert(sz <= UINT64_MAX/secsz);
                 sz *= secsz;
 
                 start = fdisk_partition_get_start(p);
-                assert_se(start <= UINT64_MAX/secsz);
+                assert(start <= UINT64_MAX/secsz);
                 start *= secsz;
 
                 partno = fdisk_partition_get_partno(p);
@@ -4799,7 +4799,7 @@ static int determine_auto_size(Context *c) {
         uint64_t sum;
         Partition *p;
 
-        assert_se(c);
+        assert(c);
 
         sum = round_up_size(GPT_METADATA_SIZE, 4096);
 
@@ -4981,7 +4981,7 @@ static int run(int argc, char *argv[]) {
                 /* Flush out everything again, and let's grow the file first, then start fresh */
                 context_unload_partition_table(context);
 
-                assert_se(arg_size != UINT64_MAX);
+                assert(arg_size != UINT64_MAX);
                 r = resize_backing_fd(
                                 node,
                                 &backing_fd,

src/shared/varlink.c

@@ -1660,6 +1660,7 @@ int varlink_errorb(Varlink *v, const char *error_id, ...) {
 }
 
 int varlink_error_invalid_parameter(Varlink *v, JsonVariant *parameters) {
+        int r;
 
         assert_return(v, -EINVAL);
         assert_return(parameters, -EINVAL);
@@ -1669,13 +1670,33 @@ int varlink_error_invalid_parameter(Varlink *v, JsonVariant *parameters) {
          * variant in which case we'll pull out the first key. The latter mode is useful in functions that
          * don't expect any arguments. */
 
-        if (json_variant_is_string(parameters))
+        /* varlink_error(...) expects a json object as the third parameter. Passing a string variant causes
-                return varlink_error(v, VARLINK_ERROR_INVALID_PARAMETER, parameters);
+         * parameter sanitization to fail, and it returns -EINVAL. */
+
+        if (json_variant_is_string(parameters)) {
+                _cleanup_(json_variant_unrefp) JsonVariant *parameters_obj = NULL;
+
+                r = json_build(&parameters_obj,
+                                JSON_BUILD_OBJECT(
+                                        JSON_BUILD_PAIR("parameter", JSON_BUILD_VARIANT(parameters))));
+                if (r < 0)
+                        return r;
+
+                return varlink_error(v, VARLINK_ERROR_INVALID_PARAMETER, parameters_obj);
+        }
 
         if (json_variant_is_object(parameters) &&
-            json_variant_elements(parameters) > 0)
+            json_variant_elements(parameters) > 0) {
-                return varlink_error(v, VARLINK_ERROR_INVALID_PARAMETER,
+                _cleanup_(json_variant_unrefp) JsonVariant *parameters_obj = NULL;
-                                     json_variant_by_index(parameters, 0));
+
+                r = json_build(&parameters_obj,
+                                JSON_BUILD_OBJECT(
+                                        JSON_BUILD_PAIR("parameter", JSON_BUILD_VARIANT(json_variant_by_index(parameters, 0)))));
+                if (r < 0)
+                        return r;
+
+                return varlink_error(v, VARLINK_ERROR_INVALID_PARAMETER, parameters_obj);
+        }
 
         return -EINVAL;
 }

src/systemd/sd-radv.h

@@ -59,8 +59,7 @@ int sd_radv_set_other_information(sd_radv *ra, int other);
 int sd_radv_set_preference(sd_radv *ra, unsigned preference);
 int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p);
 int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p);
-sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra, const struct in6_addr *prefix,
+void sd_radv_remove_prefix(sd_radv *ra, const struct in6_addr *prefix, unsigned char prefixlen);
-                                      unsigned char prefixlen);
 int sd_radv_set_rdnss(sd_radv *ra, uint32_t lifetime,
                       const struct in6_addr *dns, size_t n_dns);
 int sd_radv_set_dnssl(sd_radv *ra, uint32_t lifetime, char **search_list);

src/timedate/timedated.c

@@ -102,15 +102,18 @@ static void unit_status_info_clear(UnitStatusInfo *p) {
         p->active_state = mfree(p->active_state);
 }
 
-static void unit_status_info_free(UnitStatusInfo *p) {
+static UnitStatusInfo *unit_status_info_free(UnitStatusInfo *p) {
-        assert(p);
+        if (!p)
+                return NULL;
 
         unit_status_info_clear(p);
         free(p->name);
         free(p->path);
-        free(p);
+        return mfree(p);
 }
 
+DEFINE_TRIVIAL_CLEANUP_FUNC(UnitStatusInfo*, unit_status_info_free);
+
 static void context_clear(Context *c) {
         UnitStatusInfo *p;
 
@@ -129,8 +132,13 @@ static void context_clear(Context *c) {
 }
 
 static int context_add_ntp_service(Context *c, const char *s, const char *source) {
+        _cleanup_(unit_status_info_freep) UnitStatusInfo *unit = NULL;
         UnitStatusInfo *u;
 
+        assert(c);
+        assert(s);
+        assert(source);
+
         if (!unit_name_is_valid(s, UNIT_NAME_PLAIN))
                 return -EINVAL;
 
@@ -139,18 +147,17 @@ static int context_add_ntp_service(Context *c, const char *s, const char *source
                 if (streq(u->name, s))
                         return 0;
 
-        u = new0(UnitStatusInfo, 1);
+        unit = new0(UnitStatusInfo, 1);
-        if (!u)
+        if (!unit)
                 return -ENOMEM;
 
-        u->name = strdup(s);
+        unit->name = strdup(s);
-        if (!u->name) {
+        if (!unit->name)
-                free(u);
                 return -ENOMEM;
-        }
 
-        LIST_APPEND(units, c->units, u);
+        LIST_APPEND(units, c->units, unit);
-        log_unit_debug(u, "added from %s.", source);
+        log_unit_debug(unit, "added from %s.", source);
+        TAKE_PTR(unit);
 
         return 0;
 }