Re: [PATCH] mtd: phram: fix name leak when phram_setup() fails

[Zhihao Cheng]

在 2026/10/18 7:57, Palla Raghunath 写道:
> parse_err() is a macro that prints the message and then does "return 1".
> phram_setup() still uses it after parse_name() has allocated the device
> name, with a "goto error" right after each call. That goto is never
> reached, so the kfree(name) under the error label never runs and the
> name leaks every time one of these checks fails.
>
> syzbot hit this by writing "p0,0x8000000000000000,0" to
> /sys/module/phram/parameters/phram. The zero length fails the
> erasesize/len check and the "p0" string is leaked.
>
> Use pr_err() for these five messages so we really jump to the error
> label and free the name. This also means the real error code goes back
> to the caller instead of 1, which sysfs returned to userspace as if one
> byte had been written.
>
> Fixes: 49c64df88057 ("mtd: phram: fix a double free issue in error path")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: syzbot+c9d8bc9f8ccc03799e1f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=c9d8bc9f8ccc03799e1f
> Signed-off-by: Palla Raghunath <raghunathpalla.0209@xxxxxxxxx>
> ---
>   drivers/mtd/devices/phram.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
> diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> index b42cadcd76b1..7aed57b31967 100644
> --- a/drivers/mtd/devices/phram.c
> +++ b/drivers/mtd/devices/phram.c
> @@ -289,34 +289,34 @@ static int phram_setup(const char *val)
>   
>   	ret = parse_num64(&start, token[1]);
>   	if (ret) {
> -		parse_err("illegal start address\n");
> +		pr_err("illegal start address\n");
>   		goto error;
>   	}
>   
>   	ret = parse_num64(&len, token[2]);
>   	if (ret) {
> -		parse_err("illegal device length\n");
> +		pr_err("illegal device length\n");
>   		goto error;
>   	}
>   
>   	if (token[3]) {
>   		ret = parse_num64(&erasesize, token[3]);
>   		if (ret) {
> -			parse_err("illegal erasesize\n");
> +			pr_err("illegal erasesize\n");
>   			goto error;
>   		}
>   	}
>   
>   	if (len == 0 || erasesize == 0 || erasesize > len
>   	    || erasesize > UINT_MAX) {
> -		parse_err("illegal erasesize or len\n");
> +		pr_err("illegal erasesize or len\n");
>   		ret = -EINVAL;
>   		goto error;
>   	}
>   
>   	div_u64_rem(len, (uint32_t)erasesize, &rem);
>   	if (rem) {
> -		parse_err("len is not multiple of erasesize\n");
> +		pr_err("len is not multiple of erasesize\n");
>   		ret = -EINVAL;
>   		goto error;
>   	}