[PATCH] mtd: Fix potential UAF for mtdswap_dev pointers

[PATCH] mtd: Fix potential UAF for mtdswap_dev pointers

[Ma Ke]

In the mtdswap_init(), if the allocations fail, the error handling
path frees d->page_buf, d->eb_data, d->revmap and d->page_data without
setting these pointers to NULL. This could lead to UAF if subsequent
error handling or device reset operations attempt to release these
pointers again.

Set d->page_buf, d->eb_data, d->revmap and d->page_data to NULL
immediately after freeing them to prevent misuse. Release immediately
and set to NULL, adhering to the 'release implies invalid' defensive
programming principle.

Found by code review.

Cc: stable@vger.kernel.org
Fixes: a32159024620 ("mtd: Add mtdswap block driver")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 drivers/mtd/mtdswap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index 680366616da2..b315dab2a914 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1318,12 +1318,16 @@ static int mtdswap_init(struct mtdswap_dev *d, unsigned int eblocks,
 
 oob_buf_fail:
 	kfree(d->page_buf);
+	d->page_buf = NULL;
 page_buf_fail:
 	vfree(d->eb_data);
+	d->eb_data = NULL;
 eb_data_fail:
 	vfree(d->revmap);
+	d->revmap = NULL;
 revmap_fail:
 	vfree(d->page_data);
+	d->page_data = NULL;
 page_data_fail:
 	printk(KERN_ERR "%s: init failed (%d)\n", MTDSWAP_PREFIX, ret);
 	return ret;
-- 
2.25.1

Re: [PATCH] mtd: Fix potential UAF for mtdswap_dev pointers

[Miquel Raynal]

Hello Ma,

On 24/02/2025 at 21:30:07 +08, Ma Ke <make24@iscas.ac.cn> wrote:

> In the mtdswap_init(), if the allocations fail, the error handling
> path frees d->page_buf, d->eb_data, d->revmap and d->page_data without
> setting these pointers to NULL. This could lead to UAF if subsequent
> error handling or device reset operations attempt to release these
> pointers again.
>
> Set d->page_buf, d->eb_data, d->revmap and d->page_data to NULL
> immediately after freeing them to prevent misuse. Release immediately
> and set to NULL, adhering to the 'release implies invalid' defensive
> programming principle.
>
> Found by code review.
>
> Cc: stable@vger.kernel.org
> Fixes: a32159024620 ("mtd: Add mtdswap block driver")

I am sorry but are you really fixing something? There are thousand of
drivers doing nothing with their freed pointers in the error path,
because they just cannot be used anymore.

Thanks,
Miquèl