From 112e6dd106773fd0bd0d32f5452e82857e6b2c03 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 07:53:31 +0900 Subject: [PATCH 1/9] sd-device-monitor: split passes_filter() into two parts --- src/libsystemd/sd-device/device-monitor.c | 51 ++++++++++++++--------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 3948e862a9..d072171beb 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -342,49 +342,62 @@ static sd_device_monitor *device_monitor_free(sd_device_monitor *m) { DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_device_monitor, sd_device_monitor, device_monitor_free); -static int passes_filter(sd_device_monitor *m, sd_device *device) { - const char *tag, *subsystem, *devtype, *s, *d = NULL; +static int check_subsystem_filter(sd_device_monitor *m, sd_device *device) { + const char *s, *subsystem, *d, *devtype = NULL; int r; assert(m); assert(device); if (hashmap_isempty(m->subsystem_filter)) - goto tag; + return true; - r = sd_device_get_subsystem(device, &s); + r = sd_device_get_subsystem(device, &subsystem); if (r < 0) return r; - r = sd_device_get_devtype(device, &d); + r = sd_device_get_devtype(device, &devtype); if (r < 0 && r != -ENOENT) return r; - HASHMAP_FOREACH_KEY(devtype, subsystem, m->subsystem_filter) { + HASHMAP_FOREACH_KEY(d, s, m->subsystem_filter) { if (!streq(s, subsystem)) continue; - if (!devtype) - goto tag; - - if (!d) - continue; - - if (streq(d, devtype)) - goto tag; + if (!d || streq_ptr(d, devtype)) + return true; } - return 0; + return false; +} + +static bool check_tag_filter(sd_device_monitor *m, sd_device *device) { + const char *tag; + + assert(m); + assert(device); -tag: if (set_isempty(m->tag_filter)) - return 1; + return true; SET_FOREACH(tag, m->tag_filter) if (sd_device_has_tag(device, tag) > 0) - return 1; + return true; - return 0; + return false; +} + +static int passes_filter(sd_device_monitor *m, sd_device *device) { + int r; + + assert(m); + assert(device); + + r = check_subsystem_filter(m, device); + if (r <= 0) + return r; + + return check_tag_filter(m, device); } int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret) { From ac790e8bfc953f6157043bb7fcc35e0833e068f4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 09:32:39 +0900 Subject: [PATCH 2/9] sd-device-enumerator: move match_sysattr() to device-util.[ch] It will be used by sd-device-monitor in later commits. --- src/libsystemd/meson.build | 1 + src/libsystemd/sd-device/device-enumerator.c | 42 ++------------------ src/libsystemd/sd-device/device-util.c | 40 +++++++++++++++++++ src/libsystemd/sd-device/device-util.h | 8 ++++ 4 files changed, 52 insertions(+), 39 deletions(-) create mode 100644 src/libsystemd/sd-device/device-util.c diff --git a/src/libsystemd/meson.build b/src/libsystemd/meson.build index 3def5655cc..f55bdcd1a5 100644 --- a/src/libsystemd/meson.build +++ b/src/libsystemd/meson.build @@ -127,6 +127,7 @@ libsystemd_sources = files(''' sd-device/device-monitor.c sd-device/device-private.c sd-device/device-private.h + sd-device/device-util.c sd-device/device-util.h sd-device/sd-device.c sd-hwdb/hwdb-internal.h diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 2434fb625e..07c0d55065 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -287,42 +287,6 @@ int device_enumerator_add_device(sd_device_enumerator *enumerator, sd_device *de return 0; } -static bool match_sysattr_value(sd_device *device, const char *sysattr, const char *match_value) { - const char *value; - - assert(device); - assert(sysattr); - - if (sd_device_get_sysattr_value(device, sysattr, &value) < 0) - return false; - - if (!match_value) - return true; - - if (fnmatch(match_value, value, 0) == 0) - return true; - - return false; -} - -static bool match_sysattr(sd_device_enumerator *enumerator, sd_device *device) { - const char *sysattr; - const char *value; - - assert(enumerator); - assert(device); - - HASHMAP_FOREACH_KEY(value, sysattr, enumerator->nomatch_sysattr) - if (match_sysattr_value(device, sysattr, value)) - return false; - - HASHMAP_FOREACH_KEY(value, sysattr, enumerator->match_sysattr) - if (!match_sysattr_value(device, sysattr, value)) - return false; - - return true; -} - static bool match_property(sd_device_enumerator *enumerator, sd_device *device) { const char *property; const char *value; @@ -479,7 +443,7 @@ static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, if (!match_property(enumerator, device)) continue; - if (!match_sysattr(enumerator, device)) + if (!device_match_sysattr(device, enumerator->match_sysattr, enumerator->nomatch_sysattr)) continue; k = device_enumerator_add_device(enumerator, device); @@ -606,7 +570,7 @@ static int enumerator_scan_devices_tag(sd_device_enumerator *enumerator, const c if (!match_property(enumerator, device)) continue; - if (!match_sysattr(enumerator, device)) + if (!device_match_sysattr(device, enumerator->match_sysattr, enumerator->nomatch_sysattr)) continue; k = device_enumerator_add_device(enumerator, device); @@ -667,7 +631,7 @@ static int parent_add_child(sd_device_enumerator *enumerator, const char *path) if (!match_property(enumerator, device)) return 0; - if (!match_sysattr(enumerator, device)) + if (!device_match_sysattr(device, enumerator->match_sysattr, enumerator->nomatch_sysattr)) return 0; r = device_enumerator_add_device(enumerator, device); diff --git a/src/libsystemd/sd-device/device-util.c b/src/libsystemd/sd-device/device-util.c new file mode 100644 index 0000000000..fe495aecf7 --- /dev/null +++ b/src/libsystemd/sd-device/device-util.c @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include + +#include "device-util.h" + +static bool device_match_sysattr_value(sd_device *device, const char *sysattr, const char *match_value) { + const char *value; + + assert(device); + assert(sysattr); + + if (sd_device_get_sysattr_value(device, sysattr, &value) < 0) + return false; + + if (!match_value) + return true; + + if (fnmatch(match_value, value, 0) == 0) + return true; + + return false; +} + +bool device_match_sysattr(sd_device *device, Hashmap *match_sysattr, Hashmap *nomatch_sysattr) { + const char *sysattr; + const char *value; + + assert(device); + + HASHMAP_FOREACH_KEY(value, sysattr, match_sysattr) + if (!device_match_sysattr_value(device, sysattr, value)) + return false; + + HASHMAP_FOREACH_KEY(value, sysattr, nomatch_sysattr) + if (device_match_sysattr_value(device, sysattr, value)) + return false; + + return true; +} diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 88c9d9881e..4dfd8c1dd6 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -1,8 +1,14 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include + #include "sd-device.h" +#include "hashmap.h" +#include "log.h" +#include "macro.h" + #define FOREACH_DEVICE_PROPERTY(device, key, value) \ for (key = sd_device_get_property_first(device, &(value)); \ key; \ @@ -64,3 +70,5 @@ #define log_device_notice_errno(device, error, ...) log_device_full_errno(device, LOG_NOTICE, error, __VA_ARGS__) #define log_device_warning_errno(device, error, ...) log_device_full_errno(device, LOG_WARNING, error, __VA_ARGS__) #define log_device_error_errno(device, error, ...) log_device_full_errno(device, LOG_ERR, error, __VA_ARGS__) + +bool device_match_sysattr(sd_device *device, Hashmap *match_sysattr, Hashmap *nomatch_sysattr); From d9b030b673576ca217ad1a1844b9996b32309eec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 09:33:04 +0900 Subject: [PATCH 3/9] sd-device-monitor: introduce sd_device_monitor_filter_add_match_sysattr() --- src/libsystemd/libsystemd.sym | 5 +++++ src/libsystemd/sd-device/device-monitor.c | 26 ++++++++++++++++++++++- src/systemd/sd-device.h | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 74eff7fcf0..0ac00081ae 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -751,3 +751,8 @@ global: sd_device_new_from_stat_rdev; sd_device_trigger; } LIBSYSTEMD_247; + +LIBSYSTEMD_249 { +global: + sd_device_monitor_filter_add_match_sysattr; +} LIBSYSTEMD_248; diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index d072171beb..b12687cc2b 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -37,6 +37,8 @@ struct sd_device_monitor { Hashmap *subsystem_filter; Set *tag_filter; + Hashmap *match_sysattr_filter; + Hashmap *nomatch_sysattr_filter; bool filter_uptodate; sd_event *event; @@ -336,6 +338,8 @@ static sd_device_monitor *device_monitor_free(sd_device_monitor *m) { hashmap_free(m->subsystem_filter); set_free(m->tag_filter); + hashmap_free(m->match_sysattr_filter); + hashmap_free(m->nomatch_sysattr_filter); return mfree(m); } @@ -397,7 +401,10 @@ static int passes_filter(sd_device_monitor *m, sd_device *device) { if (r <= 0) return r; - return check_tag_filter(m, device); + if (!check_tag_filter(m, device)) + return false; + + return device_match_sysattr(device, m->match_sysattr_filter, m->nomatch_sysattr_filter); } int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret) { @@ -760,6 +767,21 @@ _public_ int sd_device_monitor_filter_add_match_tag(sd_device_monitor *m, const return r; } +_public_ int sd_device_monitor_filter_add_match_sysattr(sd_device_monitor *m, const char *sysattr, const char *value, int match) { + Hashmap **hashmap; + + assert_return(m, -EINVAL); + assert_return(sysattr, -EINVAL); + + if (match) + hashmap = &m->match_sysattr_filter; + else + hashmap = &m->nomatch_sysattr_filter; + + /* TODO: unset m->filter_uptodate on success when we support this filter on BPF. */ + return hashmap_put_strdup_full(hashmap, &trivial_hash_ops_free_free, sysattr, value); +} + _public_ int sd_device_monitor_filter_remove(sd_device_monitor *m) { static const struct sock_fprog filter = { 0, NULL }; @@ -767,6 +789,8 @@ _public_ int sd_device_monitor_filter_remove(sd_device_monitor *m) { m->subsystem_filter = hashmap_free(m->subsystem_filter); m->tag_filter = set_free(m->tag_filter); + m->match_sysattr_filter = hashmap_free(m->match_sysattr_filter); + m->nomatch_sysattr_filter = hashmap_free(m->nomatch_sysattr_filter); if (setsockopt(m->sock, SOL_SOCKET, SO_DETACH_FILTER, &filter, sizeof(filter)) < 0) return -errno; diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 310fcaa278..d7f78d8270 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -136,6 +136,7 @@ int sd_device_monitor_stop(sd_device_monitor *m); int sd_device_monitor_filter_add_match_subsystem_devtype(sd_device_monitor *m, const char *subsystem, const char *devtype); int sd_device_monitor_filter_add_match_tag(sd_device_monitor *m, const char *tag); +int sd_device_monitor_filter_add_match_sysattr(sd_device_monitor *m, const char *sysattr, const char *value, int match); int sd_device_monitor_filter_update(sd_device_monitor *m); int sd_device_monitor_filter_remove(sd_device_monitor *m); From bcfe746ba73631e12130d0eea569a28a1d21f692 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 09:53:50 +0900 Subject: [PATCH 4/9] sd-device-enumerator: also move match_parent() to device-util.[ch] --- src/libsystemd/sd-device/device-enumerator.c | 22 ++----------------- src/libsystemd/sd-device/device-util.c | 23 ++++++++++++++++++++ src/libsystemd/sd-device/device-util.h | 2 ++ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 07c0d55065..a290dbcc9c 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -331,24 +331,6 @@ static bool match_tag(sd_device_enumerator *enumerator, sd_device *device) { return true; } -static bool match_parent(sd_device_enumerator *enumerator, sd_device *device) { - const char *syspath_parent, *syspath; - - assert(enumerator); - assert(device); - - if (set_isempty(enumerator->match_parent)) - return true; - - assert_se(sd_device_get_syspath(device, &syspath) >= 0); - - SET_FOREACH(syspath_parent, enumerator->match_parent) - if (path_startswith(syspath, syspath_parent)) - return true; - - return false; -} - static bool match_sysname(sd_device_enumerator *enumerator, const char *sysname) { const char *sysname_match; @@ -434,7 +416,7 @@ static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, sd_device_get_ifindex(device, NULL) >= 0)) continue; - if (!match_parent(enumerator, device)) + if (!device_match_parent(device, enumerator->match_parent, NULL)) continue; if (!match_tag(enumerator, device)) @@ -564,7 +546,7 @@ static int enumerator_scan_devices_tag(sd_device_enumerator *enumerator, const c if (!match_sysname(enumerator, sysname)) continue; - if (!match_parent(enumerator, device)) + if (!device_match_parent(device, enumerator->match_parent, NULL)) continue; if (!match_property(enumerator, device)) diff --git a/src/libsystemd/sd-device/device-util.c b/src/libsystemd/sd-device/device-util.c index fe495aecf7..616c16c1fc 100644 --- a/src/libsystemd/sd-device/device-util.c +++ b/src/libsystemd/sd-device/device-util.c @@ -3,6 +3,7 @@ #include #include "device-util.h" +#include "path-util.h" static bool device_match_sysattr_value(sd_device *device, const char *sysattr, const char *match_value) { const char *value; @@ -38,3 +39,25 @@ bool device_match_sysattr(sd_device *device, Hashmap *match_sysattr, Hashmap *no return true; } + +bool device_match_parent(sd_device *device, Set *match_parent, Set *nomatch_parent) { + const char *syspath_parent, *syspath; + + assert(device); + + if (sd_device_get_syspath(device, &syspath) < 0) + return false; + + SET_FOREACH(syspath_parent, nomatch_parent) + if (path_startswith(syspath, syspath_parent)) + return false; + + if (set_isempty(match_parent)) + return true; + + SET_FOREACH(syspath_parent, match_parent) + if (path_startswith(syspath, syspath_parent)) + return true; + + return false; +} diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 4dfd8c1dd6..026bdcd644 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -8,6 +8,7 @@ #include "hashmap.h" #include "log.h" #include "macro.h" +#include "set.h" #define FOREACH_DEVICE_PROPERTY(device, key, value) \ for (key = sd_device_get_property_first(device, &(value)); \ @@ -72,3 +73,4 @@ #define log_device_error_errno(device, error, ...) log_device_full_errno(device, LOG_ERR, error, __VA_ARGS__) bool device_match_sysattr(sd_device *device, Hashmap *match_sysattr, Hashmap *nomatch_sysattr); +bool device_match_parent(sd_device *device, Set *match_parent, Set *nomatch_parent); From b8a0edbb2a27a3392653bb898629fe20efcef402 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 09:54:55 +0900 Subject: [PATCH 5/9] sd-device-monitor: introduce sd_device_monitor_filter_add_match_parent() --- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-device/device-monitor.c | 32 ++++++++++++++++++++++- src/systemd/sd-device.h | 1 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 0ac00081ae..b025b55026 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -755,4 +755,5 @@ global: LIBSYSTEMD_249 { global: sd_device_monitor_filter_add_match_sysattr; + sd_device_monitor_filter_add_match_parent; } LIBSYSTEMD_248; diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index b12687cc2b..b485e3e2b6 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -39,6 +39,8 @@ struct sd_device_monitor { Set *tag_filter; Hashmap *match_sysattr_filter; Hashmap *nomatch_sysattr_filter; + Set *match_parent_filter; + Set *nomatch_parent_filter; bool filter_uptodate; sd_event *event; @@ -340,6 +342,8 @@ static sd_device_monitor *device_monitor_free(sd_device_monitor *m) { set_free(m->tag_filter); hashmap_free(m->match_sysattr_filter); hashmap_free(m->nomatch_sysattr_filter); + set_free(m->match_parent_filter); + set_free(m->nomatch_parent_filter); return mfree(m); } @@ -404,7 +408,10 @@ static int passes_filter(sd_device_monitor *m, sd_device *device) { if (!check_tag_filter(m, device)) return false; - return device_match_sysattr(device, m->match_sysattr_filter, m->nomatch_sysattr_filter); + if (!device_match_sysattr(device, m->match_sysattr_filter, m->nomatch_sysattr_filter)) + return false; + + return device_match_parent(device, m->match_parent_filter, m->nomatch_parent_filter); } int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret) { @@ -782,6 +789,27 @@ _public_ int sd_device_monitor_filter_add_match_sysattr(sd_device_monitor *m, co return hashmap_put_strdup_full(hashmap, &trivial_hash_ops_free_free, sysattr, value); } +_public_ int sd_device_monitor_filter_add_match_parent(sd_device_monitor *m, sd_device *device, int match) { + const char *syspath; + Set **set; + int r; + + assert_return(m, -EINVAL); + assert_return(device, -EINVAL); + + r = sd_device_get_syspath(device, &syspath); + if (r < 0) + return r; + + if (match) + set = &m->match_parent_filter; + else + set = &m->nomatch_parent_filter; + + /* TODO: unset m->filter_uptodate on success when we support this filter on BPF. */ + return set_put_strdup(set, syspath); +} + _public_ int sd_device_monitor_filter_remove(sd_device_monitor *m) { static const struct sock_fprog filter = { 0, NULL }; @@ -791,6 +819,8 @@ _public_ int sd_device_monitor_filter_remove(sd_device_monitor *m) { m->tag_filter = set_free(m->tag_filter); m->match_sysattr_filter = hashmap_free(m->match_sysattr_filter); m->nomatch_sysattr_filter = hashmap_free(m->nomatch_sysattr_filter); + m->match_parent_filter = set_free(m->match_parent_filter); + m->nomatch_parent_filter = set_free(m->nomatch_parent_filter); if (setsockopt(m->sock, SOL_SOCKET, SO_DETACH_FILTER, &filter, sizeof(filter)) < 0) return -errno; diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index d7f78d8270..e760e03f35 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -137,6 +137,7 @@ int sd_device_monitor_stop(sd_device_monitor *m); int sd_device_monitor_filter_add_match_subsystem_devtype(sd_device_monitor *m, const char *subsystem, const char *devtype); int sd_device_monitor_filter_add_match_tag(sd_device_monitor *m, const char *tag); int sd_device_monitor_filter_add_match_sysattr(sd_device_monitor *m, const char *sysattr, const char *value, int match); +int sd_device_monitor_filter_add_match_parent(sd_device_monitor *m, sd_device *device, int match); int sd_device_monitor_filter_update(sd_device_monitor *m); int sd_device_monitor_filter_remove(sd_device_monitor *m); From def366933ce6d17a916ca25ccd3ef0df9b4a9ab7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 24 Feb 2021 14:05:03 +0900 Subject: [PATCH 6/9] test: add tests for filters of sd-device-monitor --- .../sd-device/test-sd-device-monitor.c | 137 +++++++++++++++++- 1 file changed, 133 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-device/test-sd-device-monitor.c b/src/libsystemd/sd-device/test-sd-device-monitor.c index fddd1c152c..ae973cdba3 100644 --- a/src/libsystemd/sd-device/test-sd-device-monitor.c +++ b/src/libsystemd/sd-device/test-sd-device-monitor.c @@ -95,10 +95,10 @@ static void test_send_receive_one(sd_device *device, bool subsystem_filter, bool static void test_subsystem_filter(sd_device *device) { _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; - const char *syspath, *subsystem, *p, *s; + const char *syspath, *subsystem; sd_device *d; - log_info("/* %s */", __func__); + log_device_info(device, "/* %s */", __func__); assert_se(sd_device_get_syspath(device, &syspath) >= 0); assert_se(sd_device_get_subsystem(device, &subsystem) >= 0); @@ -116,18 +116,142 @@ static void test_subsystem_filter(sd_device *device) { assert_se(sd_device_enumerator_new(&e) >= 0); assert_se(sd_device_enumerator_add_match_subsystem(e, subsystem, false) >= 0); FOREACH_DEVICE(e, d) { + const char *p, *s; + assert_se(sd_device_get_syspath(d, &p) >= 0); assert_se(sd_device_get_subsystem(d, &s) >= 0); - log_info("Sending device subsystem:%s syspath:%s", s, p); + log_device_debug(d, "Sending device subsystem:%s syspath:%s", s, p); assert_se(device_monitor_send_device(monitor_server, monitor_client, d) >= 0); } - log_info("Sending device subsystem:%s syspath:%s", subsystem, syspath); + log_device_info(device, "Sending device subsystem:%s syspath:%s", subsystem, syspath); assert_se(device_monitor_send_device(monitor_server, monitor_client, device) >= 0); assert_se(sd_event_loop(sd_device_monitor_get_event(monitor_client)) == 100); } +static void test_tag_filter(sd_device *device) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + const char *syspath; + sd_device *d; + + log_device_info(device, "/* %s */", __func__); + + assert_se(sd_device_get_syspath(device, &syspath) >= 0); + + assert_se(device_monitor_new_full(&monitor_server, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(sd_device_monitor_start(monitor_server, NULL, NULL) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_server), "sender") >= 0); + + assert_se(device_monitor_new_full(&monitor_client, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(device_monitor_allow_unicast_sender(monitor_client, monitor_server) >= 0); + assert_se(sd_device_monitor_filter_add_match_tag(monitor_client, "TEST_SD_DEVICE_MONITOR") >= 0); + assert_se(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_client), "receiver") >= 0); + + assert_se(sd_device_enumerator_new(&e) >= 0); + FOREACH_DEVICE(e, d) { + const char *p; + + assert_se(sd_device_get_syspath(d, &p) >= 0); + + log_device_debug(d, "Sending device syspath:%s", p); + assert_se(device_monitor_send_device(monitor_server, monitor_client, d) >= 0); + } + + log_device_info(device, "Sending device syspath:%s", syspath); + assert_se(device_monitor_send_device(monitor_server, monitor_client, device) >= 0); + assert_se(sd_event_loop(sd_device_monitor_get_event(monitor_client)) == 100); + +} + +static void test_sysattr_filter(sd_device *device, const char *sysattr) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + const char *syspath, *subsystem, *sysattr_value; + sd_device *d; + + log_device_info(device, "/* %s(%s) */", __func__, sysattr); + + assert_se(sd_device_get_syspath(device, &syspath) >= 0); + assert_se(sd_device_get_subsystem(device, &subsystem) >= 0); + assert_se(sd_device_get_sysattr_value(device, sysattr, &sysattr_value) >= 0); + + assert_se(device_monitor_new_full(&monitor_server, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(sd_device_monitor_start(monitor_server, NULL, NULL) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_server), "sender") >= 0); + + assert_se(device_monitor_new_full(&monitor_client, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(device_monitor_allow_unicast_sender(monitor_client, monitor_server) >= 0); + /* The sysattr filter is not implemented in BPF yet, so the below device_monito_send_device() + * may cause EAGAIN. So, let's also filter devices with subsystem. */ + assert_se(sd_device_monitor_filter_add_match_subsystem_devtype(monitor_client, subsystem, NULL) >= 0); + assert_se(sd_device_monitor_filter_add_match_sysattr(monitor_client, sysattr, sysattr_value, true) >= 0); + assert_se(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_client), "receiver") >= 0); + + assert_se(sd_device_enumerator_new(&e) >= 0); + assert_se(sd_device_enumerator_add_match_sysattr(e, sysattr, sysattr_value, false) >= 0); + FOREACH_DEVICE(e, d) { + const char *p; + + assert_se(sd_device_get_syspath(d, &p) >= 0); + + log_device_debug(d, "Sending device syspath:%s", p); + assert_se(device_monitor_send_device(monitor_server, monitor_client, d) >= 0); + } + + log_device_info(device, "Sending device syspath:%s", syspath); + assert_se(device_monitor_send_device(monitor_server, monitor_client, device) >= 0); + assert_se(sd_event_loop(sd_device_monitor_get_event(monitor_client)) == 100); + +} + +static void test_parent_filter(sd_device *device) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + const char *syspath, *subsystem; + sd_device *parent, *d; + int r; + + log_device_info(device, "/* %s */", __func__); + + assert_se(sd_device_get_syspath(device, &syspath) >= 0); + assert_se(sd_device_get_subsystem(device, &subsystem) >= 0); + r = sd_device_get_parent(device, &parent); + if (r < 0) + return (void) log_device_info(device, "Device does not have parent, skipping."); + + assert_se(device_monitor_new_full(&monitor_server, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(sd_device_monitor_start(monitor_server, NULL, NULL) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_server), "sender") >= 0); + + assert_se(device_monitor_new_full(&monitor_client, MONITOR_GROUP_NONE, -1) >= 0); + assert_se(device_monitor_allow_unicast_sender(monitor_client, monitor_server) >= 0); + /* The parent filter is not implemented in BPF yet, so the below device_monito_send_device() + * may cause EAGAIN. So, let's also filter devices with subsystem. */ + assert_se(sd_device_monitor_filter_add_match_subsystem_devtype(monitor_client, subsystem, NULL) >= 0); + assert_se(sd_device_monitor_filter_add_match_parent(monitor_client, parent, true) >= 0); + assert_se(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath) >= 0); + assert_se(sd_event_source_set_description(sd_device_monitor_get_event_source(monitor_client), "receiver") >= 0); + + assert_se(sd_device_enumerator_new(&e) >= 0); + FOREACH_DEVICE(e, d) { + const char *p; + + assert_se(sd_device_get_syspath(d, &p) >= 0); + + log_device_debug(d, "Sending device syspath:%s", p); + assert_se(device_monitor_send_device(monitor_server, monitor_client, d) >= 0); + } + + log_device_info(device, "Sending device syspath:%s", syspath); + assert_se(device_monitor_send_device(monitor_server, monitor_client, device) >= 0); + assert_se(sd_event_loop(sd_device_monitor_get_event(monitor_client)) == 100); + +} + static void test_sd_device_monitor_filter_remove(sd_device *device) { _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; const char *syspath; @@ -184,6 +308,7 @@ int main(int argc, char *argv[]) { assert_se(sd_device_new_from_syspath(&loopback, "/sys/class/net/lo") >= 0); assert_se(device_add_property(loopback, "ACTION", "add") >= 0); assert_se(device_add_property(loopback, "SEQNUM", "10") >= 0); + assert_se(device_add_tag(loopback, "TEST_SD_DEVICE_MONITOR", true) >= 0); test_send_receive_one(loopback, false, false, false); test_send_receive_one(loopback, true, false, false); @@ -194,6 +319,8 @@ int main(int argc, char *argv[]) { test_send_receive_one(loopback, true, true, true); test_subsystem_filter(loopback); + test_tag_filter(loopback); + test_sysattr_filter(loopback, "ifindex"); test_sd_device_monitor_filter_remove(loopback); test_device_copy_properties(loopback); @@ -214,5 +341,7 @@ int main(int argc, char *argv[]) { test_send_receive_one(sda, false, true, true); test_send_receive_one(sda, true, true, true); + test_parent_filter(sda); + return 0; } From 210e1cd6e6730367d4d104f919047c3decd6b75f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 11:00:19 +0900 Subject: [PATCH 7/9] dissect-image: filter out enumerated or triggered devices without "partition" sysattr This also adds more filters for device enumerator and monitor. These newly added filters should be mostly redundant. But this hides spurious error in sd_device_get_sysattr_value(). See, https://github.com/systemd/systemd/pull/18684#discussion_r579700977 --- src/shared/dissect-image.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index fabf3b2e2e..fb8df16ebc 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -127,10 +127,18 @@ static int enumerator_for_parent(sd_device *d, sd_device_enumerator **ret) { if (r < 0) return r; + r = sd_device_enumerator_add_match_subsystem(e, "block", true); + if (r < 0) + return r; + r = sd_device_enumerator_add_match_parent(e, d); if (r < 0) return r; + r = sd_device_enumerator_add_match_sysattr(e, "partition", NULL, true); + if (r < 0) + return r; + *ret = TAKE_PTR(e); return 0; } @@ -151,9 +159,6 @@ static int device_is_partition(sd_device *d, blkid_partition pp) { return false; r = sd_device_get_sysattr_value(d, "partition", &v); - if (r == -ENOENT || /* Not a partition device */ - ERRNO_IS_PRIVILEGE(r)) /* Not ready to access? */ - return false; if (r < 0) return r; r = safe_atoi(v, &partno); @@ -313,6 +318,14 @@ static int wait_for_partition_device( if (r < 0) return r; + r = sd_device_monitor_filter_add_match_parent(monitor, parent, true); + if (r < 0) + return r; + + r = sd_device_monitor_filter_add_match_sysattr(monitor, "partition", NULL, true); + if (r < 0) + return r; + r = sd_device_monitor_attach_event(monitor, event); if (r < 0) return r; From 11368b694e0918721af3f0917f05283fe0285b8e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 11:16:18 +0900 Subject: [PATCH 8/9] dissect-image: also check devtype in device_is_partition() This should be mostly redundant. Just for safety. --- src/shared/dissect-image.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index fb8df16ebc..4a76902b07 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -147,15 +147,18 @@ static int device_is_partition(sd_device *d, blkid_partition pp) { blkid_loff_t bsize, bstart; uint64_t size, start; int partno, bpartno, r; - const char *ss, *v; + const char *v; assert(d); assert(pp); - r = sd_device_get_subsystem(d, &ss); + r = sd_device_get_subsystem(d, &v); if (r < 0) return r; - if (!streq(ss, "block")) + if (!streq(v, "block")) + return false; + + if (sd_device_get_devtype(d, &v) < 0 || !streq(v, "partition")) return false; r = sd_device_get_sysattr_value(d, "partition", &v); From 0a8f9bc6bb8f708a2d3fafd5e0e9a700145747c4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 21 Feb 2021 11:11:28 +0900 Subject: [PATCH 9/9] dissect-image: move parent device check into device_is_partition() Checking parent for enumerated devices is mostly redundant. Just for safety. --- src/shared/dissect-image.c | 42 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 4a76902b07..ce8a683bd6 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -143,13 +143,15 @@ static int enumerator_for_parent(sd_device *d, sd_device_enumerator **ret) { return 0; } -static int device_is_partition(sd_device *d, blkid_partition pp) { +static int device_is_partition(sd_device *d, sd_device *expected_parent, blkid_partition pp) { + const char *v, *parent_syspath, *expected_parent_syspath; blkid_loff_t bsize, bstart; uint64_t size, start; int partno, bpartno, r; - const char *v; + sd_device *parent; assert(d); + assert(expected_parent); assert(pp); r = sd_device_get_subsystem(d, &v); @@ -161,6 +163,21 @@ static int device_is_partition(sd_device *d, blkid_partition pp) { if (sd_device_get_devtype(d, &v) < 0 || !streq(v, "partition")) return false; + r = sd_device_get_parent(d, &parent); + if (r < 0) + return false; /* Doesn't have a parent? No relevant to us */ + + r = sd_device_get_syspath(parent, &parent_syspath); /* Check parent of device of this action */ + if (r < 0) + return r; + + r = sd_device_get_syspath(expected_parent, &expected_parent_syspath); /* Check parent of device we are looking for */ + if (r < 0) + return r; + + if (!path_equal(parent_syspath, expected_parent_syspath)) + return false; /* Has a different parent than what we need, not interesting to us */ + r = sd_device_get_sysattr_value(d, "partition", &v); if (r < 0) return r; @@ -227,7 +244,7 @@ static int find_partition( return r; FOREACH_DEVICE(e, q) { - r = device_is_partition(q, pp); + r = device_is_partition(q, parent, pp); if (r < 0) return r; if (r > 0) { @@ -250,9 +267,7 @@ static inline void wait_data_done(struct wait_data *d) { } static int device_monitor_handler(sd_device_monitor *monitor, sd_device *device, void *userdata) { - const char *parent1_path, *parent2_path; struct wait_data *w = userdata; - sd_device *pp; int r; assert(w); @@ -260,22 +275,7 @@ static int device_monitor_handler(sd_device_monitor *monitor, sd_device *device, if (device_for_action(device, SD_DEVICE_REMOVE)) return 0; - r = sd_device_get_parent(device, &pp); - if (r < 0) - return 0; /* Doesn't have a parent? No relevant to us */ - - r = sd_device_get_syspath(pp, &parent1_path); /* Check parent of device of this action */ - if (r < 0) - goto finish; - - r = sd_device_get_syspath(w->parent_device, &parent2_path); /* Check parent of device we are looking for */ - if (r < 0) - goto finish; - - if (!path_equal(parent1_path, parent2_path)) - return 0; /* Has a different parent than what we need, not interesting to us */ - - r = device_is_partition(device, w->blkidp); + r = device_is_partition(device, w->parent_device, w->blkidp); if (r < 0) goto finish; if (r == 0) /* Not the one we need */