From 36e34057a202d389263e98030fbd775b28b28af6 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Wed, 6 Aug 2014 07:57:43 +0200 Subject: [PATCH 1/3] sd-bus: Remove bus arg from bus_verify_polkit_async_registry_free() It's unneccessary, not used, and complicates callers of the function. --- src/hostname/hostnamed.c | 6 +++--- src/libsystemd/sd-bus/bus-util.c | 2 +- src/libsystemd/sd-bus/bus-util.h | 2 +- src/locale/localed.c | 6 +++--- src/login/logind.c | 2 +- src/timedate/timedated.c | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 4acdb72b12..a6f440f4e1 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -68,11 +68,11 @@ static void context_reset(Context *c) { } } -static void context_free(Context *c, sd_bus *bus) { +static void context_free(Context *c) { assert(c); context_reset(c); - bus_verify_polkit_async_registry_free(bus, c->polkit_registry); + bus_verify_polkit_async_registry_free(c->polkit_registry); } static int context_read_data(Context *c) { @@ -721,7 +721,7 @@ int main(int argc, char *argv[]) { } finish: - context_free(&context, bus); + context_free(&context); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c index 32c536813d..37f5c4620c 100644 --- a/src/libsystemd/sd-bus/bus-util.c +++ b/src/libsystemd/sd-bus/bus-util.c @@ -452,7 +452,7 @@ int bus_verify_polkit_async( return -EACCES; } -void bus_verify_polkit_async_registry_free(sd_bus *bus, Hashmap *registry) { +void bus_verify_polkit_async_registry_free(Hashmap *registry) { #ifdef ENABLE_POLKIT AsyncPolkitQuery *q; diff --git a/src/libsystemd/sd-bus/bus-util.h b/src/libsystemd/sd-bus/bus-util.h index af50553926..b3b52af24c 100644 --- a/src/libsystemd/sd-bus/bus-util.h +++ b/src/libsystemd/sd-bus/bus-util.h @@ -65,7 +65,7 @@ int bus_check_peercred(sd_bus *c); int bus_verify_polkit(sd_bus *bus, sd_bus_message *m, const char *action, bool interactive, bool *_challenge, sd_bus_error *e); int bus_verify_polkit_async(sd_bus *bus, Hashmap **registry, sd_bus_message *m, const char *action, bool interactive, sd_bus_error *error, sd_bus_message_handler_t callback, void *userdata); -void bus_verify_polkit_async_registry_free(sd_bus *bus, Hashmap *registry); +void bus_verify_polkit_async_registry_free(Hashmap *registry); int bus_open_system_systemd(sd_bus **_bus); int bus_open_user_systemd(sd_bus **_bus); diff --git a/src/locale/localed.c b/src/locale/localed.c index 3cfc2ea4d3..4d7d02b9a3 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -131,12 +131,12 @@ static void context_free_locale(Context *c) { free_and_replace(&c->locale[p], NULL); } -static void context_free(Context *c, sd_bus *bus) { +static void context_free(Context *c) { context_free_locale(c); context_free_x11(c); context_free_vconsole(c); - bus_verify_polkit_async_registry_free(bus, c->polkit_registry); + bus_verify_polkit_async_registry_free(c->polkit_registry); }; static void locale_simplify(Context *c) { @@ -1160,7 +1160,7 @@ int main(int argc, char *argv[]) { } finish: - context_free(&context, bus); + context_free(&context); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/login/logind.c b/src/login/logind.c index 8ec8105357..006c56ae51 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -163,7 +163,7 @@ void manager_free(Manager *m) { if (m->udev) udev_unref(m->udev); - bus_verify_polkit_async_registry_free(m->bus, m->polkit_registry); + bus_verify_polkit_async_registry_free(m->polkit_registry); sd_bus_unref(m->bus); sd_event_unref(m->event); diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 791e2b436f..1efbf3386e 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -52,11 +52,11 @@ typedef struct Context { Hashmap *polkit_registry; } Context; -static void context_free(Context *c, sd_bus *bus) { +static void context_free(Context *c) { assert(c); free(c->zone); - bus_verify_polkit_async_registry_free(bus, c->polkit_registry); + bus_verify_polkit_async_registry_free(c->polkit_registry); } static int context_read_data(Context *c) { @@ -727,7 +727,7 @@ int main(int argc, char *argv[]) { } finish: - context_free(&context, bus); + context_free(&context); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } From 2ea31e5b13448fd7a9757da4bcd1de04a151ac3f Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Wed, 6 Aug 2014 11:34:40 +0200 Subject: [PATCH 2/3] core: Common code for DBus methods that Cancel a job Both ofs.Job.Cancel() and ofs.Manager.CancelJob() now use same implementation. So we can add caller verify logic appropriately. --- src/core/dbus-job.c | 4 ++-- src/core/dbus-job.h | 2 ++ src/core/dbus-manager.c | 9 ++------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index 8e4ffc977d..542006257d 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -52,7 +52,7 @@ static int property_get_unit( return sd_bus_message_append(reply, "(so)", j->unit->id, p); } -static int method_cancel(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) { +int bus_job_method_cancel(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) { Job *j = userdata; int r; @@ -71,7 +71,7 @@ static int method_cancel(sd_bus *bus, sd_bus_message *message, void *userdata, s const sd_bus_vtable bus_job_vtable[] = { SD_BUS_VTABLE_START(0), - SD_BUS_METHOD("Cancel", NULL, NULL, method_cancel, 0), + SD_BUS_METHOD("Cancel", NULL, NULL, bus_job_method_cancel, 0), SD_BUS_PROPERTY("Id", "u", NULL, offsetof(Job, id), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Unit", "(so)", property_get_unit, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("JobType", "s", property_get_type, offsetof(Job, type), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/dbus-job.h b/src/core/dbus-job.h index d1d0f6ddf0..6c2fc0789c 100644 --- a/src/core/dbus-job.h +++ b/src/core/dbus-job.h @@ -26,5 +26,7 @@ extern const sd_bus_vtable bus_job_vtable[]; +int bus_job_method_cancel(sd_bus *bus, sd_bus_message *message, void *job, sd_bus_error *error); + void bus_job_send_change_signal(Job *j); void bus_job_send_removed_signal(Job *j); diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index ffef0c7d05..e4d6369c69 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -34,6 +34,7 @@ #include "architecture.h" #include "env-util.h" #include "dbus.h" +#include "dbus-job.h" #include "dbus-manager.h" #include "dbus-unit.h" #include "dbus-snapshot.h" @@ -685,13 +686,7 @@ static int method_cancel_job(sd_bus *bus, sd_bus_message *message, void *userdat if (!j) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_JOB, "Job %u does not exist.", (unsigned) id); - r = selinux_unit_access_check(j->unit, message, "stop", error); - if (r < 0) - return r; - - job_finish_and_invalidate(j, JOB_CANCELED, true); - - return sd_bus_reply_method_return(message, NULL); + return bus_job_method_cancel(bus, message, j, error); } static int method_clear_jobs(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) { From b39a2770ba55637da80e2e389222c59dbea73507 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Wed, 6 Aug 2014 11:53:00 +0200 Subject: [PATCH 3/3] core: Rename Job.subscribed field to Job.clients This reflects how this field will be used, to not only track where to send signals, but also which callers (other than root) are allowed to call DBus methods on the Job. --- src/core/dbus-job.c | 4 ++-- src/core/dbus-unit.c | 6 +++--- src/core/dbus.c | 4 ++-- src/core/job.c | 10 +++++----- src/core/job.h | 12 +++++++++--- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c index 542006257d..747f633f85 100644 --- a/src/core/dbus-job.c +++ b/src/core/dbus-job.c @@ -132,7 +132,7 @@ void bus_job_send_change_signal(Job *j) { j->in_dbus_queue = false; } - r = bus_foreach_bus(j->manager, j->subscribed, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j); + r = bus_foreach_bus(j->manager, j->clients, j->sent_dbus_new_signal ? send_changed_signal : send_new_signal, j); if (r < 0) log_debug("Failed to send job change signal for %u: %s", j->id, strerror(-r)); @@ -176,7 +176,7 @@ void bus_job_send_removed_signal(Job *j) { if (!j->sent_dbus_new_signal) bus_job_send_change_signal(j); - r = bus_foreach_bus(j->manager, j->subscribed, send_removed_signal, j); + r = bus_foreach_bus(j->manager, j->clients, send_removed_signal, j); if (r < 0) log_debug("Failed to send job remove signal for %u: %s", j->id, strerror(-r)); } diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 8f23fe76ce..2132f59faa 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -758,13 +758,13 @@ int bus_unit_queue_job( return r; if (bus == u->manager->api_bus) { - if (!j->subscribed) { - r = sd_bus_track_new(bus, &j->subscribed, NULL, NULL); + if (!j->clients) { + r = sd_bus_track_new(bus, &j->clients, NULL, NULL); if (r < 0) return r; } - r = sd_bus_track_add_sender(j->subscribed, message); + r = sd_bus_track_add_sender(j->clients, message); if (r < 0) return r; } diff --git a/src/core/dbus.c b/src/core/dbus.c index fb8e4963e1..2f0b4abb5e 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -1048,8 +1048,8 @@ static void destroy_bus(Manager *m, sd_bus **bus) { m->subscribed = sd_bus_track_unref(m->subscribed); HASHMAP_FOREACH(j, m->jobs, i) - if (j->subscribed && sd_bus_track_get_bus(j->subscribed) == *bus) - j->subscribed = sd_bus_track_unref(j->subscribed); + if (j->clients && sd_bus_track_get_bus(j->clients) == *bus) + j->clients = sd_bus_track_unref(j->clients); /* Get rid of queued message on this bus */ if (m->queued_message_bus == *bus) { diff --git a/src/core/job.c b/src/core/job.c index dc4f44150c..5e4987f4e2 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -90,8 +90,8 @@ void job_free(Job *j) { sd_event_source_unref(j->timer_event_source); - sd_bus_track_unref(j->subscribed); - strv_free(j->deserialized_subscribed); + sd_bus_track_unref(j->clients); + strv_free(j->deserialized_clients); free(j); } @@ -937,7 +937,7 @@ int job_serialize(Job *j, FILE *f, FDSet *fds) { if (j->begin_usec > 0) fprintf(f, "job-begin="USEC_FMT"\n", j->begin_usec); - bus_track_serialize(j->subscribed, f); + bus_track_serialize(j->clients, f); /* End marker */ fputc('\n', f); @@ -1043,7 +1043,7 @@ int job_deserialize(Job *j, FILE *f, FDSet *fds) { } else if (streq(l, "subscribed")) { - if (strv_extend(&j->deserialized_subscribed, v) < 0) + if (strv_extend(&j->deserialized_clients, v) < 0) return log_oom(); } } @@ -1056,7 +1056,7 @@ int job_coldplug(Job *j) { /* After deserialization is complete and the bus connection * set up again, let's start watching our subscribers again */ - r = bus_track_coldplug(j->manager, &j->subscribed, &j->deserialized_subscribed); + r = bus_track_coldplug(j->manager, &j->clients, &j->deserialized_clients); if (r < 0) return r; diff --git a/src/core/job.h b/src/core/job.h index 30d41d9edd..1e7c61b04f 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -145,9 +145,15 @@ struct Job { sd_event_source *timer_event_source; usec_t begin_usec; - /* There can be more than one client, because of job merging. */ - sd_bus_track *subscribed; - char **deserialized_subscribed; + /* + * This tracks where to send signals, and also which clients + * are allowed to call DBus methods on the job (other than + * root). + * + * There can be more than one client, because of job merging. + */ + sd_bus_track *clients; + char **deserialized_clients; JobResult result;