Re: [PATCH 01/58] mtd: clear OTP provider pointers after unregistering them

[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:
- [Critical] mtd: memory leak and use-after-free on
  mtd_device_parse_register() failure [1]
--

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

mtd: clear OTP provider pointers after unregistering them

This commit centralizes OTP nvmem provider removal into
mtd_otp_nvmem_remove() and clears the provider pointers after
unregistering them to prevent duplicate unregistrations on error and
device teardown paths.

> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 8efa2d88556e9..558a605ba7495 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c

[ ... ]

> @@ -1180,10 +1188,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
>  	}
>  
>  out:
> -	if (ret) {
> -		nvmem_unregister(mtd->otp_user_nvmem);
> -		nvmem_unregister(mtd->otp_factory_nvmem);
> -	}
> +	if (ret)
> +		mtd_otp_nvmem_remove(mtd);
>  
>  	if (ret && device_is_registered(&mtd->dev)) {
>  		err = del_mtd_device(mtd);

[Severity: Critical]
This problem wasn't introduced by this patch, but can a failure in
mtd_virt_concat_create_join() leak partitions and leave dangling pointers
in concat->subdev[] [1]?

In mtd_device_parse_register(), parse_mtd_partitions() or
add_mtd_partitions() runs before mtd_virt_concat_create_join(). When
mtd_virt_concat_add() claims a child partition, it stores the pointer in
concat->subdev[] and add_mtd_partitions() skips adding the child to
parent->partitions.

If mtd_virt_concat_create_join() then fails and jumps to out, this unwind
path returns without calling mtd_virt_concat_destroy() or
del_mtd_partitions(mtd). Because callers free the master mtd_info when
mtd_device_parse_register() fails, registered partitions are leaked and
concat->subdev[] retains partitions whose parent pointers reference the
freed master mtd_info, causing a use-after-free via mtd_get_master() when
a later device registration calls mtd_virt_concat_create_join(). (The
err_del_partitions path in add_mtd_partitions() similarly calls only
del_mtd_partitions(), leaving any partitions already claimed by
mtd_virt_concat_add() in concat->subdev[].)

[1] https://sashiko.dev/#/bug/linux-eb184d91-0c17-4015-99a4-10b54060a747

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-submit-rawnand-retained-handle-lifetime-v1-v1-0-0181ef0cc4c8@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic