Re: [PATCH 1/2] mtd: rawnand: macronix: serialize OTP access with NAND operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] mtd: rawnand: macronix: clobbered error return in
  __macronix_30lfxg18ac_rw_otp() [1]
--

commit c6d2a3c4fdab02c0f7d81bd92a1998898aaa2b83
Author: James Hilliard <james.hilliard1@gmail.com>

mtd: rawnand: macronix: serialize OTP access with NAND operations

This commit exposes nand_get_device() and nand_release_device() within the
raw NAND core and uses them to serialize Macronix OTP transactions with
normal NAND operations and suspend handling.

> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
> index 03237310852cb..4fe7036ae9921 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -380,6 +380,7 @@ static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd,
>  	int ret;
>  
>  	nand = mtd_to_nand(mtd);
> +	nand_get_device(nand);
>  	nand_select_target(nand, 0);
>  
>  	ret = macronix_30lfxg18ac_otp_enable(nand);
> @@ -426,6 +427,7 @@ static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd,
>  			write ? "write" : "read");
>  
>  	nand_deselect_target(nand);
> +	nand_release_device(nand);
>  
>  	return ret;
>  }

[Severity: Medium]
This problem wasn't introduced by this patch, but doesn't the out_otp
cleanup path clobber ret [1] with the result of
macronix_30lfxg18ac_otp_disable()?

drivers/mtd/nand/raw/nand_macronix.c:__macronix_30lfxg18ac_rw_otp() {
	...
	ret = macronix_30lfxg18ac_otp_enable(nand);
	if (ret)
		goto out_otp;
	...
out_otp:
	if (ret)
		dev_err(&mtd->dev, "failed to perform OTP IO: %i\n", ret);

	ret = macronix_30lfxg18ac_otp_disable(nand);
	if (ret)
		dev_err(&mtd->dev, "failed to leave OTP mode after %s\n",
			write ? "write" : "read");

	nand_deselect_target(nand);
	nand_release_device(nand);

	return ret;
}

When macronix_30lfxg18ac_otp_enable(), nand_prog_page_op(), or
nand_read_page_op() fails and macronix_30lfxg18ac_otp_disable() then
succeeds, __macronix_30lfxg18ac_rw_otp() returns 0 without updating
*retlen, causing callers such as mtd_read_user_prot_reg() to treat OTP I/O
errors as 0-byte transfers.  Jumping to out_otp when
macronix_30lfxg18ac_otp_enable() fails also calls
macronix_30lfxg18ac_otp_disable() even though OTP mode was never enabled.

[1] https://sashiko.dev/#/bug/linux-c639ea49-b980-45c7-9d77-b0e264970125

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-submit-rawnand-operation-locking-v1-v1-0-565010fc5f8b@gmail.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/