[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aCXg9t1QWR5jgpqX@casper.infradead.org>
Date: Thu, 15 May 2025 13:41:26 +0100
From: Matthew Wilcox <willy@...radead.org>
To: caius.zone@...oud.com
Cc: phillip@...ashfs.org.uk, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, Caius Zone <zone@...sast.com>
Subject: Re: [PATCH] squashfs: fix NULL pointer dereference in
bio_alloc_clone failure path
On Thu, May 15, 2025 at 08:01:54PM +0800, caius.zone@...oud.com wrote:
> diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
> index 2dc730800f44..b00a71f8933c 100644
> --- a/fs/squashfs/block.c
> +++ b/fs/squashfs/block.c
> @@ -115,6 +115,9 @@ static int squashfs_bio_read_cached(struct bio *fullbio,
> struct bio *new = bio_alloc_clone(bdev, fullbio,
> GFP_NOIO, &fs_bio_set);
>
> + if (!new)
> + return -ENOMEM;
> +
> if (bio) {
> bio_trim(bio, start_idx * PAGE_SECTORS,
> (end_idx - start_idx) * PAGE_SECTORS);
This is definitely wrong. The check for 'new' being NULL should be
after the submit_bio() otherwise we'll leak the previous bio.
But then we need to not call bio_chain() ... so I think this ends up
looking like:
@@ -115,12 +115,15 @@ static int squashfs_bio_read_cached(struct bio *fullbio,
struct bio *new = bio_alloc_clone(bdev, fullbio,
GFP_NOIO, &fs_bio_set);
+ if (new)
+ bio_chain(bio, new);
if (bio) {
bio_trim(bio, start_idx * PAGE_SECTORS,
(end_idx - start_idx) * PAGE_SECTORS);
- bio_chain(bio, new);
submit_bio(bio);
}
+ if (!new)
+ return -ENOMEM;
bio = new;
start_idx = idx;
... but that might not be right either. We might need to break out of
the loop, rather than returning and do the submit_bio_wait() followed by
the bio_put().
Powered by blists - more mailing lists