On Thu, 2020-09-17 at 21:26 -0500, Benjamin Marzinski wrote: > On Tue, Sep 15, 2020 at 12:38:27PM +0800, lixiaokeng wrote: > > Hi, > > The udev* function may return NULL,and it will be > > dereferenced in str* and sscanf func. For example, > > there is a coredump caused in add func, which show in > > be7a043(commit id) in upstream-queue. We check the > > return value to avoid dereference NULL. > > > > repo: openSUSE/multipath-tools > > repo link: https://github.com/openSUSE/multipath-tools > > branch: upstream-queue > > > For the set > Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> Well, the set got the logic wrong in 3 out of 6 patches :-) In general, as these fixes are all very similar, I would prefer bundling them together into one patch, and I would like to make sure (and assert in the commit message) that the set fixes *all* possible NULL pointer dereferences related to accessing udev properties (I haven't checked whether this is the case for the set). Also, while I'd ack this set if the logic was correct, I think that in a way it's papering over the actual problem. udev_device_get_XYZ() functions fail *only* if the underlying sysfs directory has vanished (well, perhaps for out-of-memory, too, but let's put that aside for a moment). It that case, we should actually not just return an error code - we should realize that there's a problem with the device (it probably has been deleted from the system and shouldn't be used any more in any way), and that we need to deal with it somehow. This is a fundamental problem for all user space programs that use udev_devices for more than a few CPU cycles. That doesn't mean the set is wrong, but we should keep this in mind. I have something cooking for a more complete solution, which isn't ready yet. Finally, @lixiaokeng, please check your inbox once more. You still haven't fixed #11/14 of your previous series. Regards, Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
