Re: [systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
On Fri, 30.08.13 17:22, Colin Guthrie (gm...@colin.guthr.ie) wrote: 'Twas brillig, and David Herrmann at 30/08/13 14:50 did gyre and gimble: +subsystem = subsystem ? : ; + zomg!!11eleven! This makes perfect sense but somehow this is the first time I've seen this syntax before where the value between ? and : is inherited from the test itself... nice! For what it's worth the two other uses of this syntax in the systemd codebase don't have a space between the ? and : (dunno if this qualifies for a coding standard tho'!) /me will now go and craw back under his rock... btw, for this specific usecase we have a macro strempty(), which hides the ternary operator... We generally prefer using that... Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
On Fri, Aug 30, 2013 at 3:50 PM, David Herrmann dh.herrm...@gmail.com wrote: udev_device_get_subsystem() may return NULL if no subsystem could be figured out by libudev. This might be due to OOM or if the device disconnected between the udev_device_new() call and udev_device_get_subsystem(). Therefore, we need to handle subsystem==NULL safely. Yeah, seems we never handled that properly. The lazy evaluation of properties can return NULL if the device is removed between creating it and asking for the values. Instead of testing for it in each helper, we treat subsystem==NULL as empty subsystem in match_subsystem(). Applied. Thanks, Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
udev_device_get_subsystem() may return NULL if no subsystem could be figured out by libudev. This might be due to OOM or if the device disconnected between the udev_device_new() call and udev_device_get_subsystem(). Therefore, we need to handle subsystem==NULL safely. Instead of testing for it in each helper, we treat subsystem==NULL as empty subsystem in match_subsystem(). Backtrace of udev_enumerate with an input-device disconnecting in exactly this time-frame: (gdb) bt #0 0x7569dc24 in strnlen () from /usr/lib/libc.so.6 #1 0x756d9e04 in fnmatch@@GLIBC_2.2.5 () from /usr/lib/libc.so.6 #2 0x75beb83d in match_subsystem (udev_enumerate=0x7a05f0, subsystem=0x0) at src/libudev/libudev-enumerate.c:727 #3 0x75bebb30 in parent_add_child (enumerate=enumerate@entry=0x7a05f0, path=optimized out) at src/libudev/libudev-enumerate.c:834 #4 0x75bebc3f in parent_crawl_children (enumerate=enumerate@entry=0x7a05f0, path=0x7a56b0 /sys/devices/shortened/input/input97, maxdepth=maxdepth@entry=254) at src/libudev/libudev-enumerate.c:866 #5 0x75bebc54 in parent_crawl_children (enumerate=enumerate@entry=0x7a05f0, path=0x79e8c0 /sys/devices/shortened/input, maxdepth=maxdepth@entry=255) at src/libudev/libudev-enumerate.c:868 #6 0x75bebc54 in parent_crawl_children (enumerate=enumerate@entry=0x7a05f0, path=path@entry=0x753190 /sys/devices/shortened, maxdepth=maxdepth@entry=256) at src/libudev/libudev-enumerate.c:868 #7 0x75bec7df in scan_devices_children (enumerate=0x7a05f0) at src/libudev/libudev-enumerate.c:882 #8 udev_enumerate_scan_devices (udev_enumerate=udev_enumerate@entry=0x7a05f0) at src/libudev/libudev-enumerate.c:919 #9 0x75df8777 in random_caller () at some/file.c:181 --- src/libudev/libudev-enumerate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libudev/libudev-enumerate.c b/src/libudev/libudev-enumerate.c index 5ccaabd..7065719 100644 --- a/src/libudev/libudev-enumerate.c +++ b/src/libudev/libudev-enumerate.c @@ -718,6 +718,8 @@ static bool match_subsystem(struct udev_enumerate *udev_enumerate, const char *s { struct udev_list_entry *list_entry; +subsystem = subsystem ? : ; + udev_list_entry_foreach(list_entry, udev_list_get_entry(udev_enumerate-subsystem_nomatch_list)) { if (fnmatch(udev_list_entry_get_name(list_entry), subsystem, 0) == 0) return false; -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
'Twas brillig, and David Herrmann at 30/08/13 14:50 did gyre and gimble: +subsystem = subsystem ? : ; + zomg!!11eleven! This makes perfect sense but somehow this is the first time I've seen this syntax before where the value between ? and : is inherited from the test itself... nice! For what it's worth the two other uses of this syntax in the systemd codebase don't have a space between the ? and : (dunno if this qualifies for a coding standard tho'!) /me will now go and craw back under his rock... Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
On Fri, Aug 30, 2013 at 1:22 PM, Colin Guthrie gm...@colin.guthr.ie wrote: 'Twas brillig, and David Herrmann at 30/08/13 14:50 did gyre and gimble: +subsystem = subsystem ? : ; + zomg!!11eleven! This makes perfect sense but somehow this is the first time I've seen this syntax before where the value between ? and : is inherited from the test itself... nice! this is an extension: http://gcc.gnu.org/onlinedocs/gcc/Conditionals.html As far as I know it's supported both by clang and gcc Lucas De Marchi ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel