Re: [PATCH] direct-io: prevent possible race condition on bio_list

[Matthew Wilcox <willy@infradead.org>]

On Sat, Feb 26, 2022 at 11:17:48PM +0100, Niels Dossche wrote:
> Prevent bio_list from changing in the while loop condition such that the
> body of the loop won't execute with a potentially NULL pointer for
> bio_list, which causes a NULL dereference later on.

Is this something you've seen happen, or something you think might
happen?

> Signed-off-by: Niels Dossche <dossche.niels@gmail.com>
> ---
>  fs/direct-io.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 654443558047..806f05407019 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -545,19 +545,22 @@ static inline int dio_bio_reap(struct dio *dio, struct dio_submit *sdio)
>  	int ret = 0;
>  
>  	if (sdio->reap_counter++ >= 64) {
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&dio->bio_lock, flags);
>  		while (dio->bio_list) {
> -			unsigned long flags;
>  			struct bio *bio;
>  			int ret2;
>  
> -			spin_lock_irqsave(&dio->bio_lock, flags);
>  			bio = dio->bio_list;
>  			dio->bio_list = bio->bi_private;
>  			spin_unlock_irqrestore(&dio->bio_lock, flags);
>  			ret2 = blk_status_to_errno(dio_bio_complete(dio, bio));
>  			if (ret == 0)
>  				ret = ret2;
> +			spin_lock_irqsave(&dio->bio_lock, flags);
>  		}
> +		spin_unlock_irqrestore(&dio->bio_lock, flags);
>  		sdio->reap_counter = 0;
>  	}
>  	return ret;
> -- 
> 2.35.1
>