Skip to content

Commit

Permalink
DW-1381 fix BSOD when volume is removed during IO.
Browse files Browse the repository at this point in the history 
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()
  • Loading branch information
@HyunjunLee
HyunjunLee committed Jan 23, 2017
1 parent d236498 commit 7705b82
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 12 deletions.
10 changes: 5 additions & 5 deletions wdrbd9/disp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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)) {

Expand Down
28 changes: 26 additions & 2 deletions wdrbd9/drbd_windows.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down Expand Up @@ -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);
Expand All @@ -2527,14 +2530,32 @@ 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;

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)
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion wdrbd9/drbd_windows.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion wdrbd9/ops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 2 additions & 3 deletions wdrbd9/sub.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7705b82

Please sign in to comment.