From 7705b823e344ee5b6db19e17443fc209e7e54600 Mon Sep 17 00:00:00 2001 From: Hyunjun Lee Date: Mon, 23 Jan 2017 15:09:29 +0900 Subject: [PATCH] DW-1381 fix BSOD when volume is removed during IO. drbd sometimes get read irp after volume is removed, and gets crashed while getting drbd device via volume extension. get_device_with_vol_ext() assumes that volume extension has valid block devices, which is wrong. added checking couple of conditions to avoid accessing nullified block device. 1. get_device_with_vol_ext() fails if the member dev of volume extension is NULL. 2. get_device_with_vol_ext() fails if it fails to acquire volume remove lock. (it means the volume is being removed) 3. setting dev as NULL has been moved to delete_drbd_block_device() from mvolRemoveDevice() --- wdrbd9/disp.c | 10 +++++----- wdrbd9/drbd_windows.c | 28 ++++++++++++++++++++++++++-- wdrbd9/drbd_windows.h | 2 +- wdrbd9/ops.c | 2 +- wdrbd9/sub.c | 5 ++--- 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/wdrbd9/disp.c b/wdrbd9/disp.c index 3be93796..aef5081e 100644 --- a/wdrbd9/disp.c +++ b/wdrbd9/disp.c @@ -413,7 +413,7 @@ mvolCreate(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (VolumeExtension->Active) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, TRUE); // DW-1300: prevent mounting volume when device went diskless. if (device && ((R_PRIMARY != device->resource->role[NOW]) || (device->resource->bPreDismountLock == TRUE) || device->disk_state[NOW] == D_DISKLESS)) // V9 { @@ -476,7 +476,7 @@ mvolFlush(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (g_mj_flush_buffers_filter && VolumeExtension->Active) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, TRUE); if (device) { PMVOL_THREAD pThreadInfo; pThreadInfo = &VolumeExtension->WorkThreadInfo; @@ -520,7 +520,7 @@ mvolSystemControl(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (VolumeExtension->Active) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, TRUE); // DW-1300: prevent mounting volume when device is failed or below. if (device && device->resource->bTempAllowMount == FALSE && ((R_PRIMARY != device->resource->role[NOW]) || (device->resource->bPreDismountLock == TRUE) || device->disk_state[NOW] <= D_FAILED)) // V9 { @@ -565,7 +565,7 @@ mvolRead(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (VolumeExtension->Active) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, TRUE); // DW-1363: prevent mounting volume when device is failed or below. if (device && ((R_PRIMARY == device->resource->role[0]) && (device->resource->bPreDismountLock == FALSE) && device->disk_state[NOW] > D_FAILED || device->resource->bTempAllowMount == TRUE)) { @@ -640,7 +640,7 @@ mvolWrite(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (VolumeExtension->Active) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, TRUE); // DW-1363: prevent writing when device is failed or below. if (device && device->resource && (device->resource->role[NOW] == R_PRIMARY) && (device->resource->bPreSecondaryLock == FALSE) && (device->disk_state[NOW] > D_FAILED)) { diff --git a/wdrbd9/drbd_windows.c b/wdrbd9/drbd_windows.c index 58752448..5e00001f 100644 --- a/wdrbd9/drbd_windows.c +++ b/wdrbd9/drbd_windows.c @@ -2237,7 +2237,7 @@ void query_targetdev(PVOLUME_EXTENSION pvext) !RtlEqualUnicodeString(&pvext->MountPoint, &new_name, TRUE)) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(pvext); + struct drbd_device *device = get_device_with_vol_ext(pvext, TRUE); if (device && get_ldev_if_state(device, D_NEGOTIATING)) { @@ -2518,6 +2518,9 @@ void delete_drbd_block_device(struct kref *kref) ObDereferenceObject(bdev->bd_disk->pDeviceExtension->DeviceObject); bdev->bd_disk->pDeviceExtension->DeviceObject = NULL; + // DW-1381: set dev as NULL not to access from this volume extension since it's being deleted. + bdev->bd_disk->pDeviceExtension->dev = NULL; + blk_cleanup_queue(bdev->bd_disk->queue); put_disk(bdev->bd_disk); @@ -2527,7 +2530,7 @@ void delete_drbd_block_device(struct kref *kref) } // get device with volume extension in safe, user should put ref when no longer use device. -struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext) +struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext, bool bCheckRemoveLock) { unsigned char oldIRQL = 0; struct drbd_device *device = NULL; @@ -2535,6 +2538,24 @@ struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext) if (KeGetCurrentIrql() > DISPATCH_LEVEL) return NULL; + // DW-1381: dev is set as NULL when block device is destroyed. + if (!pvext->dev) + { + WDRBD_ERROR("failed to get drbd device since pvext->dev is NULL\n"); + return NULL; + } + + // DW-1381: check if device is removed already. + if (bCheckRemoveLock) + { + NTSTATUS status = IoAcquireRemoveLock(&pvext->RemoveLock, NULL); + if (!NT_SUCCESS(status)) + { + WDRBD_INFO("failed to acquire remove lock with status:0x%x, return NULL\n", status); + return NULL; + } + } + oldIRQL = ExAcquireSpinLockShared(&pvext->dev->bd_disk->drbd_device_ref_lock); device = pvext->dev->bd_disk->drbd_device; if (device) @@ -2548,6 +2569,9 @@ struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext) } ExReleaseSpinLockShared(&pvext->dev->bd_disk->drbd_device_ref_lock, oldIRQL); + if (bCheckRemoveLock) + IoReleaseRemoveLock(&pvext->RemoveLock, NULL); + return device; } diff --git a/wdrbd9/drbd_windows.h b/wdrbd9/drbd_windows.h index 5799e76c..8de4ac98 100644 --- a/wdrbd9/drbd_windows.h +++ b/wdrbd9/drbd_windows.h @@ -1327,7 +1327,7 @@ extern void NTAPI NetlinkServerThread(PVOID p); extern struct block_device * create_drbd_block_device(IN OUT PVOLUME_EXTENSION pvext); extern void delete_drbd_block_device(struct kref *kref); // DW-1300 -extern struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext); +extern struct drbd_device *get_device_with_vol_ext(PVOLUME_EXTENSION pvext, bool bCheckRemoveLock); extern BOOLEAN do_add_minor(unsigned int minor); extern void drbdFreeDev(PVOLUME_EXTENSION pDeviceExtension); extern void query_targetdev(PVOLUME_EXTENSION pvext); diff --git a/wdrbd9/ops.c b/wdrbd9/ops.c index 6b0d72c3..dacc0832 100644 --- a/wdrbd9/ops.c +++ b/wdrbd9/ops.c @@ -267,7 +267,7 @@ IOCTL_MountVolume(PDEVICE_OBJECT DeviceObject, PIRP Irp, PULONG ReturnLength) } // DW-1300: get device and get reference. - device = get_device_with_vol_ext(pvext); + device = get_device_with_vol_ext(pvext, TRUE); if (pvext->WorkThreadInfo.Active && device) { sprintf(Message, "%wZ volume is handling by drbd. Failed to mount", diff --git a/wdrbd9/sub.c b/wdrbd9/sub.c index e0b62d6f..bbf01e21 100644 --- a/wdrbd9/sub.c +++ b/wdrbd9/sub.c @@ -162,7 +162,7 @@ mvolRemoveDevice(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) if (VolumeExtension->dev) { // DW-1300: get device and get reference. - struct drbd_device *device = get_device_with_vol_ext(VolumeExtension); + struct drbd_device *device = get_device_with_vol_ext(VolumeExtension, FALSE); if (device) { if (get_disk_state2(device) >= D_INCONSISTENT) @@ -180,7 +180,6 @@ mvolRemoveDevice(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) // DW-1109: put ref count that's been set as 1 when initialized, in add device routine. // deleting block device can be defered if drbd device is using. blkdev_put(VolumeExtension->dev, 0); - VolumeExtension->dev = NULL; } // DW-1277: check volume type we marked when drbd attaches. @@ -317,7 +316,7 @@ mvolReadWriteDevice(PVOLUME_EXTENSION VolumeExtension, PIRP Irp, ULONG Io) } // DW-1300: get device and get reference. - device = get_device_with_vol_ext(VolumeExtension); + device = get_device_with_vol_ext(VolumeExtension, TRUE); if (device/* && (mdev->state.role == R_PRIMARY)*/) { struct splitInfo *splitInfo = 0; ULONG io_id = 0;