Re: [PATCH] drbd: fix potential silent data corruption

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Apr 26, 2021 at 06:17:08PM +0200, Christoph Böhmwalder wrote:
> From: Lars Ellenberg <lars.ellenberg@linbit.com>
> 
> Scenario:
> ---------
> 
> bio chain generated by blk_queue_split().
> Some split bio fails and propagates its error status to the "parent" bio.
> But then the (last part of the) parent bio itself completes without error.
> 
> We would clobber the already recorded error status with BLK_STS_OK,
> causing silent data corruption.
> 
> Reproducer:
> -----------
> 
> How to trigger this in the real world within seconds:
> 
> DRBD on top of degraded parity raid,
> small stripe_cache_size, large read_ahead setting.
> Drop page cache (sysctl vm.drop_caches=1, fadvise "DONTNEED",
> umount and mount again, "reboot").
> 
> Cause significant read ahead.
> 
> Large read ahead request is split by blk_queue_split().
> Parts of the read ahead that are already in the stripe cache,
> or find an available stripe cache to use, can be serviced.
> Parts of the read ahead that would need "too much work",
> would need to wait for a "stripe_head" to become available,
> are rejected immediately.
> 
> For larger read ahead requests that are split in many pieces, it is very
> likely that some "splits" will be serviced, but then the stripe cache is
> exhausted/busy, and the remaining ones will be rejected.
> 
> Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
> Signed-off-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
> Cc: <stable@vger.kernel.org> # 4.13.x
> ---
>  drivers/block/drbd/drbd_req.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
> index 9398c2c2cb2d..a384a58de1fd 100644
> --- a/drivers/block/drbd/drbd_req.c
> +++ b/drivers/block/drbd/drbd_req.c
> @@ -180,7 +180,8 @@ void start_new_tl_epoch(struct drbd_connection *connection)
>  void complete_master_bio(struct drbd_device *device,
>  		struct bio_and_error *m)
>  {
> -	m->bio->bi_status = errno_to_blk_status(m->error);
> +	if (unlikely(m->error))
> +		m->bio->bi_status = errno_to_blk_status(m->error);
>  	bio_endio(m->bio);
>  	dec_ap_bio(device);
>  }
> -- 
> 2.26.3
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>