[libvirt] Potential segfault in udev driver

Matthias Bolte matthias.bolte at googlemail.com
Mon Jan 25 19:33:02 UTC 2010

2010/1/25 Dave Allan <dallan at redhat.com>:
> On 01/25/2010 06:37 AM, Daniel P. Berrange wrote:
>> On Sun, Jan 24, 2010 at 11:07:59PM +0100, Matthias Bolte wrote:
>>> udevDeviceMonitorStartup registers udevEventHandleCallback as event
>>> handle, but doesn't store the returned watch id to remove it later on.
>>> Also it's not clear to me whether the event handle should be register
>>> for the whole lifetime of the udev driver instance or just for the
>>> udevEnumerateDevices call.
>> The handler should be active for the lifetime of libvirtd, since the
>> udev driver has to detect hotplug/unplug events over time.
>>> If for example the call to udevSetupSystemDev [1] fails
>>> udevDeviceMonitorShutdown is called to cleanup, but
>>> udevEventHandleCallback is still registered and may be called when
>>> driverState is NULL again, resulting in a segfault in
>>> udevEventHandleCallback.
>>> So to solve this the udevEventHandleCallback event handle must be
>>> removed at the appropriate place.
>> Yes, sounds like its needs to be removed in the failure path there
> Matthias,
> Indeed, that's correct--can you submit a patch?
> Dave

Yes, im going to do that.

One last question. The udev driver stores the udev monitor handle in
the private data pointer of the gloval driver state variable.

    driverState->privateData = udev_monitor;

To solve the event handle problem we need to store the watch id
returned by virEventAddHandle somewhere. We could either add a new
private data struct to hold the udev_monitor pointer and the watch id,
or store the watch id variable globally side by side with the driver
state variable. I prefer the first option, because it's cleaner and
the DRV_STATE_UDEV_MONITOR define allows for changing the storage
location of the udev_monitor pointer easily.


More information about the libvir-list mailing list