openzfsonwindows / ZFSin

OpenZFS on Windows port
https://openzfsonwindows.org
1.2k stars 68 forks source link

Driver is not uninstalled properly. #293

Open arturkow2000 opened 4 years ago

arturkow2000 commented 4 years ago

After uninstalling ZFSin driver gets disabled as its NT service is gone:

[System.ServiceProcess.ServiceController]::GetDevices() | Where-Object {$_.ServiceName -match 'zfs'}

returns nothing but driver still resides in driver store:

pnputil /enum-drivers | Select-String -Pattern zfs -Context 1,4

  Published Name:     oem8.inf
> Original Name:      zfsin.inf
  Provider Name:      Jorgen Lundman
  Class Name:         Woluminy magazynu
  Class GUID:         {71a27cdd-812a-11d0-bec7-08002be2092f}
  Driver Version:     04/02/2020 8.49.35.481

Old drivers accumulate after many uninstallations/reinstallations and require manual deletion.

lundman commented 4 years ago

Does anyone know how to uninstall a driver? I've been guessing the whole way and it seems complicated and still doesn't work right.

imtiazdc commented 4 years ago

Please see related: https://github.com/openzfsonwindows/ZFSin/issues/258

Notes from our (incomplete) investigation on this issue: After the uninstallation, ZFSin.sys remains in memory. Debugging of uninstall code path revealed that there were a few device objects whose ref counts were increased by calling ObReferenceObject but corresponding ObDereferenceObject was never called.

After applying the below patch,

diff --git a/ZFSin/zfs/module/zfs/zfs_ioctl.c b/ZFSin/zfs/module/zfs/zfs_ioctl.c
old mode 100644
new mode 100755
index 05199a4..4fcb64f
--- a/ZFSin/zfs/module/zfs/zfs_ioctl.c
+++ b/ZFSin/zfs/module/zfs/zfs_ioctl.c
@@ -6784,8 +6784,10 @@ zfs_ioc_unregister_fs(void)
    if (fsDiskDeviceObject != NULL) {
        IoUnregisterFsRegistrationChange(WIN_DriverObject, DriverNotificationRoutine);
        IoUnregisterFileSystem(fsDiskDeviceObject);
+       ObDereferenceObject(fsDiskDeviceObject);
        IoDeleteDevice(fsDiskDeviceObject);
        fsDiskDeviceObject = NULL;
+       ObDereferenceObject(ioctlDeviceObject);
        IoDeleteDevice(ioctlDeviceObject);
        ioctlDeviceObject = NULL;
    }
@@ -7911,6 +7913,7 @@ zfs_attach(void)

    if (!NT_SUCCESS(ntStatus)) {
        dprintf("ZFS: Couldn't create userland symbolic link to /dev/zfs (%wZ)\n", ZFS_DEV);
+       ObDereferenceObject(ioctlDeviceObject);
        IoDeleteDevice(ioctlDeviceObject);
        return -1;
    }

we were able to unload the driver when there were no zpool and zvol created.

However, when there were any zpool/zvol created and some I/O performed, we noticed that the driver unloading actually causes an issue - I suppose there were some DPCs created which crashed due to the code getting removed from the address space on driver unloading.

We were not sure how to proceed any further after the above observation.

lundman commented 4 years ago

Oh ah - that is very interesting, thanks for looking into it.

I thought the only place that uses DPC is in vdev_disk.c (vdev_file.c) ? But I'm fairly confident you can not unload with active ZIOs, and the DPC is to handle calling zio_interrupt(). But leaks could be anywhere.

lundman commented 4 years ago

I added your patch, and it highlighted a problem with unloading a thread which I also fixed.
A step in the right direction perhaps.

vrajendra-datacore commented 3 years ago

@lundman: In function zfs_ioc_unregister_fs, the symlink ZFS_DEV_DOS has to be removed as I see the symlink being present even after uninstallation (as seen from winobj systool). Something like below

    UNICODE_STRING ntWin32NameString;
    RtlInitUnicodeString(&ntWin32NameString, ZFS_DEV_DOS);
    IoDeleteSymbolicLink(&ntWin32NameString);
lundman commented 3 years ago

ace - thank you. I'll fix it in new, and backport to the current repo.