[PATCH] pktcdvd: fix missing bio_alloc_clone NULL check in pkt_make_request_read

[PATCH] pktcdvd: fix missing bio_alloc_clone NULL check in pkt_make_request_read

[Ye Chey]

The bio_alloc_clone() call in pkt_make_request_read() lacks NULL check,
which could lead to NULL pointer dereference. Add NULL check and handle
allocation failure by calling bio_io_error().

Signed-off-by: Ye Chey <yechey@ai-sast.com>
---
 drivers/block/pktcdvd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 65b96c083..68d1f43a7 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2323,6 +2323,10 @@ static void pkt_make_request_read(struct pktcdvd_device *pd, struct bio *bio)
 {
 	struct bio *cloned_bio = bio_alloc_clone(file_bdev(pd->bdev_file), bio,
 		GFP_NOIO, &pkt_bio_set);
+	if (!cloned_bio) {
+		bio_io_error(bio);
+		return;
+	}
 	struct packet_stacked_data *psd = mempool_alloc(&psd_pool, GFP_NOIO);
 
 	psd->pd = pd;
-- 
2.44.0

Re: [PATCH] pktcdvd: fix missing bio_alloc_clone NULL check in pkt_make_request_read

[Christoph Hellwig]

On Wed, May 21, 2025 at 08:30:19PM +0800, Ye Chey wrote:
> The bio_alloc_clone() call in pkt_make_request_read() lacks NULL check,
> which could lead to NULL pointer dereference. Add NULL check and handle
> allocation failure by calling bio_io_error().

Please explain in detail how this could ever lead to a path in
bio_alloc_clone that could return NULL and how you came to that
conclusion.

Re: [PATCH] pktcdvd: fix missing bio_alloc_clone NULL check in pkt_make_request_read

[yechey]

Hi Christoph,

Thank you for your feedback.

While `bio_alloc_clone()` is called with `GFP_NOIO`, there are still
cases where such allocations can fail under memory pressure, particularly
on constrained systems or during heavy I/O.

I've noticed that some drivers, such as `dm-zoned`, perform a similar NULL
check after `bio_alloc_clone()` and handle allocation failures gracefully
by calling `bio_io_error()`. This suggests that the possibility of failure
is taken into account in other parts of the kernel.

Given that, I thought it would be prudent to include a NULL check here as
well, to avoid any risk of dereferencing a NULL pointer—even if the
probability is low.

Please let me know if you think this makes sense, or if I might have
missed something.

Best regards,
Chey

> On 22 May 2025, at 12:13 AM, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Wed, May 21, 2025 at 08:30:19PM +0800, Ye Chey wrote:
>> The bio_alloc_clone() call in pkt_make_request_read() lacks NULL check,
>> which could lead to NULL pointer dereference. Add NULL check and handle
>> allocation failure by calling bio_io_error().
> 
> Please explain in detail how this could ever lead to a path in
> bio_alloc_clone that could return NULL and how you came to that
> conclusion.