| From 64dafbc9530c10300acffc57fae3269d95fa8f93 Mon Sep 17 00:00:00 2001 |
| From: Lars Ellenberg <lars.ellenberg@linbit.com> |
| Date: Mon, 25 Jun 2018 11:39:52 +0200 |
| Subject: drbd: fix access after free |
| |
| From: Lars Ellenberg <lars.ellenberg@linbit.com> |
| |
| commit 64dafbc9530c10300acffc57fae3269d95fa8f93 upstream. |
| |
| We have |
| struct drbd_requests { ... struct bio *private_bio; ... } |
| to hold a bio clone for local submission. |
| |
| On local IO completion, we put that bio, and in case we want to use the |
| result later, we overload that member to hold the ERR_PTR() of the |
| completion result, |
| |
| Which, before v4.3, used to be the passed in "int error", |
| so we could first bio_put(), then assign. |
| |
| v4.3-rc1~100^2~21 4246a0b63bd8 block: add a bi_error field to struct bio |
| changed that: |
| bio_put(req->private_bio); |
| - req->private_bio = ERR_PTR(error); |
| + req->private_bio = ERR_PTR(bio->bi_error); |
| |
| Which introduces an access after free, |
| because it was non obvious that req->private_bio == bio. |
| |
| Impact of that was mostly unnoticable, because we only use that value |
| in a multiple-failure case, and even then map any "unexpected" error |
| code to EIO, so worst case we could potentially mask a more specific |
| error with EIO in a multiple failure case. |
| |
| Unless the pointed to memory region was unmapped, as is the case with |
| CONFIG_DEBUG_PAGEALLOC, in which case this results in |
| |
| BUG: unable to handle kernel paging request |
| |
| v4.13-rc1~70^2~75 4e4cbee93d56 block: switch bios to blk_status_t |
| changes it further to |
| bio_put(req->private_bio); |
| req->private_bio = ERR_PTR(blk_status_to_errno(bio->bi_status)); |
| |
| And blk_status_to_errno() now contains a WARN_ON_ONCE() for unexpected |
| values, which catches this "sometimes", if the memory has been reused |
| quickly enough for other things. |
| |
| Should also go into stable since 4.3, with the trivial change around 4.13. |
| |
| Cc: stable@vger.kernel.org |
| Fixes: 4246a0b63bd8 block: add a bi_error field to struct bio |
| Reported-by: Sarah Newman <srn@prgmr.com> |
| Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com> |
| Signed-off-by: Jens Axboe <axboe@kernel.dk> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/block/drbd/drbd_worker.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| --- a/drivers/block/drbd/drbd_worker.c |
| +++ b/drivers/block/drbd/drbd_worker.c |
| @@ -256,8 +256,8 @@ void drbd_request_endio(struct bio *bio) |
| } else |
| what = COMPLETED_OK; |
| |
| - bio_put(req->private_bio); |
| req->private_bio = ERR_PTR(bio->bi_error); |
| + bio_put(bio); |
| |
| /* not req_mod(), we need irqsave here! */ |
| spin_lock_irqsave(&device->resource->req_lock, flags); |