• Lars Ellenberg's avatar
    drbd: fix access after free · d6f703be
    Lars Ellenberg authored
    BugLink: https://bugs.launchpad.net/bugs/1784409
    
    commit 64dafbc9 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 4246a0b6 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 4e4cbee9 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: 4246a0b6 block: add a bi_error field to struct bio
    Reported-by: default avatarSarah Newman <srn@prgmr.com>
    Signed-off-by: default avatarLars Ellenberg <lars.ellenberg@linbit.com>
    Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
    Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
    d6f703be
drbd_worker.c 61.7 KB