Re: [PATCH v2 2/2] scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] target: rd: memory leak in rd_build_prot_space() on allocation failure
--

commit 08e2848928f98cb3a6b575947cf8d271014c318f
Author: Sanan Hasanov <sanan.hasanov@ucf.edu>
Subject: scsi: target: rd: Don't invoke the OOM killer for ramdisk pages

This commit replaces the default memory allocation flags for ramdisk backing
pages with GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN to avoid invoking
the OOM killer. The pages are requested via alloc_pages() in
rd_allocate_sgl_table().

> diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
> index 97c63b12266fb..46698300e2063 100644
> --- a/drivers/target/target_core_rd.c
> +++ b/drivers/target/target_core_rd.c
> @@ -150,7 +150,8 @@ static int rd_allocate_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *
>  						- 1;
>  
>  		for (j = 0; j < sg_per_table; j++) {
> -			pg = alloc_pages(GFP_KERNEL, 0);
> +			pg = alloc_pages(GFP_KERNEL | __GFP_RETRY_MAYFAIL |
> +					 __GFP_NOWARN, 0);
>  			if (!pg) {

[Severity: Medium]
Can this new failure path cause a regression by leaking memory in
rd_build_prot_space()?

Since this allocation uses __GFP_RETRY_MAYFAIL, it can now fail and return
-ENOMEM. When this occurs, rd_build_prot_space() propagates the error
without cleaning up the partially populated array:

drivers/target/target_core_rd.c:rd_build_prot_space() {
    ...
	rd_dev->sg_prot_array = sg_table;
	rd_dev->sg_prot_count = sg_tables;

	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0xff);
	if (rc)
		return rc;
    ...
}

This propagates the error up to the configfs handler:

drivers/target/target_core_configfs.c:pi_prot_type_store() {
    ...
		ret = dev->transport->init_prot(dev);
		if (ret) {
			da->pi_prot_type = old_prot;
			da->pi_prot_verify = (bool) da->pi_prot_type;
			return ret;
		}
    ...
}

The configfs handler restores state and returns without explicitly calling
dev->transport->free_prot(dev) to clean up the backend.

This leaves the partially populated scatterlists and pages permanently
dangling in rd_dev->sg_prot_array. If the user retries enabling the
protection type, rd_build_prot_space() unconditionally overwrites the
rd_dev->sg_prot_array pointer with a new allocation, permanently leaking
the previously allocated memory resources.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008223712.49827-1-sanan.hasanou@gmail.com?part=2