2010/1/26 David Allan <dallan@xxxxxxxxxx>:
> ---
> src/node_device/node_device_udev.c | 34 +++++++++++++---------------------
> 1 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index dcd889d..6895ac5 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1368,8 +1368,9 @@ static int udevDeviceMonitorShutdown(void)
>
> priv = driverState->privateData;
>
> - if (priv->watch != -1)
> + if (priv->watch != -1) {
> virEventRemoveHandle(priv->watch);
> + }
>
> udev_monitor = DRV_STATE_UDEV_MONITOR(driverState);
>
Due to your changes to udevDeviceMonitorStartup it is possible that
udevDeviceMonitorShutdown is called with driverState != NULL and
driverState->privateData == NULL. In this case 'if (priv->watch !=
-1)' will segfault and 'udev_monitor =
DRV_STATE_UDEV_MONITOR(driverState);' will segfault also. Changing it
like this will fix the problem:
if (priv != NULL) {
if (priv->watch != -1)
virEventRemoveHandle(priv->watch);
udev_monitor = DRV_STATE_UDEV_MONITOR(driverState);
}
> @@ -1387,6 +1388,7 @@ static int udevDeviceMonitorShutdown(void)
> virMutexDestroy(&driverState->lock);
> VIR_FREE(driverState);
> VIR_FREE(priv);
> +
> } else {
> ret = -1;
> }
> @@ -1547,28 +1549,22 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
> {
> udevPrivate *priv = NULL;
> struct udev *udev = NULL;
> - int ret = 0;
> + int ret = -1;
>
> - if (VIR_ALLOC(priv) < 0) {
> + if (VIR_ALLOC(driverState) < 0) {
> virReportOOMError(NULL);
> - ret = -1;
> goto out;
> }
>
> - priv->watch = -1;
> -
> - if (VIR_ALLOC(driverState) < 0) {
> + if (VIR_ALLOC(priv) < 0) {
> virReportOOMError(NULL);
> - VIR_FREE(priv);
> - ret = -1;
> goto out;
> }
>
> + priv->watch = -1;
> +
> if (virMutexInit(&driverState->lock) < 0) {
> VIR_ERROR0("Failed to initialize mutex for driverState");
> - VIR_FREE(priv);
> - VIR_FREE(driverState);
> - ret = -1;
priv leaks here.
> goto out;
> }
>
> @@ -1585,10 +1581,8 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
>
> priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
> if (priv->udev_monitor == NULL) {
> - VIR_FREE(priv);
priv leaks here.
Moving the driverState->privateData = priv; line directly after the
priv->watch = -1; will fix this leaks.
> nodeDeviceUnlock(driverState);
> VIR_ERROR0("udev_monitor_new_from_netlink returned NULL");
> - ret = -1;
> goto out;
> }
>
> @@ -1608,27 +1602,25 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
> priv->watch = virEventAddHandle(udev_monitor_get_fd(priv->udev_monitor),
> VIR_EVENT_HANDLE_READABLE,
> udevEventHandleCallback, NULL, NULL);
> +
> + nodeDeviceUnlock(driverState);
> +
> if (priv->watch == -1) {
> - nodeDeviceUnlock(driverState);
> - ret = -1;
> goto out;
> }
>
> - nodeDeviceUnlock(driverState);
> -
> /* Create a fictional 'computer' device to root the device tree. */
> if (udevSetupSystemDev() != 0) {
> - ret = -1;
> goto out;
> }
>
> /* Populate with known devices */
> -
> if (udevEnumerateDevices(udev) != 0) {
> - ret = -1;
> goto out;
> }
>
> + ret = 0;
> +
> out:
> if (ret == -1) {
> udevDeviceMonitorShutdown();
> --
> 1.6.5.5
>
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list