Re: [systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches

2013-09-09 Thread Lennart Poettering
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

2013-09-04 Thread Kay Sievers
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

2013-08-30 Thread David Herrmann
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

2013-08-30 Thread Colin Guthrie
'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

2013-08-30 Thread Lucas De Marchi
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