diff options
| author | Lars Ellenberg <lars.ellenberg@linbit.com> | 2017-08-29 10:20:39 +0200 |
|---|---|---|
| committer | Jens Axboe <axboe@kernel.dk> | 2017-08-29 23:34:45 +0200 |
| commit | 7c752ed3257517fc8607ab1d19fe4e86155721e3 (patch) | |
| tree | e025a9525f246ea9d78d7f95123ce5add35f516a /drivers/block/drbd/drbd_worker.c | |
| parent | drbd: new disk-option disable-write-same (diff) | |
| download | linux-7c752ed3257517fc8607ab1d19fe4e86155721e3.tar.xz linux-7c752ed3257517fc8607ab1d19fe4e86155721e3.zip | |
drbd: fix potential get_ldev/put_ldev refcount imbalance during attach
Race:
drbd_adm_attach() | async drbd_md_endio()
|
device->ldev is still NULL. |
|
drbd_md_read( |
.endio = drbd_md_endio; |
submit; |
.... |
wait for done == 1; | done = 1;
); | wake_up();
.. lot of other stuff, |
.. includeing taking and |
...giving up locks, |
.. doing further IO, |
.. stuff that takes "some time" |
| while in this context,
| this is the next statement.
| which means this context was scheduled
.. only then, finally, | away for "some time".
device->ldev = nbc; |
| if (device->ldev)
| put_ldev()
Unlikely, but possible. I was able to provoke it "reliably"
by adding an mdelay(500); after the wake_up().
Fixed by moving the if (!NULL) put_ldev() before done = 1;
Impact of the bug was that the resulting refcount imbalance
could lead to premature destruction of the object, potentially
causing a NULL pointer dereference during a subsequent detach.
Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Diffstat (limited to 'drivers/block/drbd/drbd_worker.c')
| -rw-r--r-- | drivers/block/drbd/drbd_worker.c | 8 |
1 files changed, 5 insertions, 3 deletions
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c index e48012df108a..f0717a97a42a 100644 --- a/drivers/block/drbd/drbd_worker.c +++ b/drivers/block/drbd/drbd_worker.c @@ -65,6 +65,11 @@ void drbd_md_endio(struct bio *bio) device = bio->bi_private; device->md_io.error = blk_status_to_errno(bio->bi_status); + /* special case: drbd_md_read() during drbd_adm_attach() */ + if (device->ldev) + put_ldev(device); + bio_put(bio); + /* We grabbed an extra reference in _drbd_md_sync_page_io() to be able * to timeout on the lower level device, and eventually detach from it. * If this io completion runs after that timeout expired, this @@ -79,9 +84,6 @@ void drbd_md_endio(struct bio *bio) drbd_md_put_buffer(device); device->md_io.done = 1; wake_up(&device->misc_wait); - bio_put(bio); - if (device->ldev) /* special case: drbd_md_read() during drbd_adm_attach() */ - put_ldev(device); } /* reads on behalf of the partner, |