From 901bfcd8c10a1058542c4be272e7b706b0911f02 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 20 Jan 2025 00:04:01 +0900 Subject: [PATCH 1/2] tree-wide: rely on sd_varlink_dispatch() for validating zero-argument method calls, too Follow-ups for a7cc7038ac06d2c02cbaa5a749aa47b4f7cbe786. --- src/bootctl/bootctl-reboot-to-firmware.c | 5 +++-- src/bootctl/bootctl-status.c | 5 +++-- src/core/core-varlink.c | 5 +++-- src/journal/journald-server.c | 25 ++++++++++++++++-------- src/libsystemd/sd-varlink/sd-varlink.c | 5 +++-- src/network/networkd-manager-varlink.c | 11 +++++++---- src/pcrlock/pcrlock.c | 10 ++++++---- src/udev/udev-varlink.c | 19 ++++++++++++------ 8 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/bootctl/bootctl-reboot-to-firmware.c b/src/bootctl/bootctl-reboot-to-firmware.c index 0bedd230bb..e24234aa90 100644 --- a/src/bootctl/bootctl-reboot-to-firmware.c +++ b/src/bootctl/bootctl-reboot-to-firmware.c @@ -62,8 +62,9 @@ int vl_method_set_reboot_to_firmware(sd_varlink *link, sd_json_variant *paramete int vl_method_get_reboot_to_firmware(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { int r; - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; r = efi_get_reboot_to_firmware(); if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) diff --git a/src/bootctl/bootctl-status.c b/src/bootctl/bootctl-status.c index 6bcb348935..2ca49ec485 100644 --- a/src/bootctl/bootctl-status.c +++ b/src/bootctl/bootctl-status.c @@ -876,8 +876,9 @@ int vl_method_list_boot_entries(sd_varlink *link, sd_json_variant *parameters, s assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; if (!FLAGS_SET(flags, SD_VARLINK_METHOD_MORE)) return sd_varlink_error(link, SD_VARLINK_ERROR_EXPECTED_MORE, NULL); diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c index 92cfd36e71..985bd702b8 100644 --- a/src/core/core-varlink.c +++ b/src/core/core-varlink.c @@ -325,8 +325,9 @@ static int vl_method_subscribe_managed_oom_cgroups( if (!streq(u->id, "systemd-oomd.service")) return sd_varlink_error(link, SD_VARLINK_ERROR_PERMISSION_DENIED, NULL); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; /* We only take one subscriber for this method so return an error if there's already an existing one. * This shouldn't happen since systemd-oomd is the only client of this method. */ diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 3469a90fef..16fb88324f 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -2223,8 +2223,9 @@ static int vl_method_synchronize(sd_varlink *link, sd_json_variant *parameters, assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; log_info("Received client request to sync journal."); @@ -2259,11 +2260,13 @@ static int vl_method_synchronize(sd_varlink *link, sd_json_variant *parameters, static int vl_method_rotate(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { Server *s = ASSERT_PTR(userdata); + int r; assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; log_info("Received client request to rotate journal, rotating."); server_full_rotate(s); @@ -2273,11 +2276,14 @@ static int vl_method_rotate(sd_varlink *link, sd_json_variant *parameters, sd_va static int vl_method_flush_to_var(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { Server *s = ASSERT_PTR(userdata); + int r; assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; + if (s->namespace) return sd_varlink_error(link, "io.systemd.Journal.NotSupportedByNamespaces", NULL); @@ -2289,11 +2295,14 @@ static int vl_method_flush_to_var(sd_varlink *link, sd_json_variant *parameters, static int vl_method_relinquish_var(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { Server *s = ASSERT_PTR(userdata); + int r; assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; + if (s->namespace) return sd_varlink_error(link, "io.systemd.Journal.NotSupportedByNamespaces", NULL); diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c index 8658c81f67..ffb596b378 100644 --- a/src/libsystemd/sd-varlink/sd-varlink.c +++ b/src/libsystemd/sd-varlink/sd-varlink.c @@ -1197,8 +1197,9 @@ static int generic_method_get_info( assert(link); assert(link->server); - if (sd_json_variant_elements(parameters) != 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; sd_varlink_interface *interface; HASHMAP_FOREACH(interface, link->server->interfaces) { diff --git a/src/network/networkd-manager-varlink.c b/src/network/networkd-manager-varlink.c index 6206fb6e44..3182c0094e 100644 --- a/src/network/networkd-manager-varlink.c +++ b/src/network/networkd-manager-varlink.c @@ -17,11 +17,13 @@ static int vl_method_get_states(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { Manager *m = ASSERT_PTR(userdata); + int r; assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; return sd_varlink_replybo( link, @@ -40,8 +42,9 @@ static int vl_method_get_namespace_id(sd_varlink *link, sd_json_variant *paramet assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; /* Network namespaces have two identifiers: the inode number (which all namespace types have), and * the "nsid" (aka the "cookie"), which only network namespaces know as a concept, and which is not diff --git a/src/pcrlock/pcrlock.c b/src/pcrlock/pcrlock.c index 72b13f8fac..2d46b31b58 100644 --- a/src/pcrlock/pcrlock.c +++ b/src/pcrlock/pcrlock.c @@ -5269,8 +5269,9 @@ static int vl_method_read_event_log(sd_varlink *link, sd_json_variant *parameter assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; el = event_log_new(); if (!el) @@ -5332,8 +5333,9 @@ static int vl_method_remove_policy(sd_varlink *link, sd_json_variant *parameters assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; r = remove_policy(); if (r < 0) diff --git a/src/udev/udev-varlink.c b/src/udev/udev-varlink.c index 8faadb8bcf..d95bfcdd91 100644 --- a/src/udev/udev-varlink.c +++ b/src/udev/udev-varlink.c @@ -11,10 +11,13 @@ #define UDEV_VARLINK_ADDRESS "/run/udev/io.systemd.Udev" static int vl_method_reload(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + int r; + assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; log_debug("Received io.systemd.service.Reload()"); manager_reload(userdata, /* force = */ true); @@ -87,8 +90,9 @@ static int vl_method_start_stop_exec_queue(sd_varlink *link, sd_json_variant *pa assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; r = sd_varlink_get_current_method(link, &method); if (r < 0) @@ -100,10 +104,13 @@ static int vl_method_start_stop_exec_queue(sd_varlink *link, sd_json_variant *pa } static int vl_method_exit(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + int r; + assert(link); - if (sd_json_variant_elements(parameters) > 0) - return sd_varlink_error_invalid_parameter(link, parameters); + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; /* Refuse further connections. */ _unused_ _cleanup_(sd_varlink_flush_close_unrefp) sd_varlink *v = sd_varlink_ref(link); From d919a155c03c3036eed47be5421e7e2d0593db1c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 20 Jan 2025 00:08:07 +0900 Subject: [PATCH 2/2] udev-varlink: add io.systemd.service.GetEnvironment There is no reason to not support the method in udevd. Follow-up for c8127075ae22ad0f64d3072f400818f6e1a9ab48. --- src/udev/udev-varlink.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/udev/udev-varlink.c b/src/udev/udev-varlink.c index d95bfcdd91..5d1a27219f 100644 --- a/src/udev/udev-varlink.c +++ b/src/udev/udev-varlink.c @@ -156,14 +156,15 @@ int manager_start_varlink_server(Manager *manager) { r = sd_varlink_server_bind_method_many( v, - "io.systemd.service.Ping", varlink_method_ping, - "io.systemd.service.Reload", vl_method_reload, - "io.systemd.service.SetLogLevel", vl_method_set_log_level, - "io.systemd.Udev.SetChildrenMax", vl_method_set_children_max, - "io.systemd.Udev.SetEnvironment", vl_method_set_environment, - "io.systemd.Udev.StartExecQueue", vl_method_start_stop_exec_queue, - "io.systemd.Udev.StopExecQueue", vl_method_start_stop_exec_queue, - "io.systemd.Udev.Exit", vl_method_exit); + "io.systemd.service.Ping", varlink_method_ping, + "io.systemd.service.Reload", vl_method_reload, + "io.systemd.service.SetLogLevel", vl_method_set_log_level, + "io.systemd.service.GetEnvironment", varlink_method_get_environment, + "io.systemd.Udev.SetChildrenMax", vl_method_set_children_max, + "io.systemd.Udev.SetEnvironment", vl_method_set_environment, + "io.systemd.Udev.StartExecQueue", vl_method_start_stop_exec_queue, + "io.systemd.Udev.StopExecQueue", vl_method_start_stop_exec_queue, + "io.systemd.Udev.Exit", vl_method_exit); if (r < 0) return log_error_errno(r, "Failed to bind Varlink methods: %m");