[PATCH] mtd: Fix refcounting with MTD_PARTITIONED_MASTER

[Usyskin, Alexander]

> 
> The logic is way too convoluted, let's clean the kref_get/put section to
> clarify what this block does, hopefully solving the refcounting issue
> when using CONFIG_MTD_PARTITIONED_MASTER at the same time:
> - Iterate through all the parent mtd devices
> - Grab a reference over them all but the master
> - Only grab the master whith CONFIG_MTD_PARTITIONED_MASTER
> Same logic must apply in the put path, otherwise it would be broken.
> 
> Cc: Tomas Winkler <tomas.winkler at intel.com>
> Cc: Alexander Usyskin <alexander.usyskin at intel.com>
> Cc: Zhang Xiaoxu <zhangxiaoxu5 at huawei.com>
> Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> 
> ---
> 
> Hello,
> 
> Alexander, Zhang, please test this version, it is close to what Zhang
> produced while not strictly identical. This compile-tested only, please
> check on you side whether it fixes the refcounting issue with and
> without PARTITIONED_MASTER, as well as with Kasan.
> 
> Alexander, maybe there is something else to fix, in all cases the logic
> here was broken so let's start by this one and see if we need something
> else on top.
> 
> Thanks,
> Miquèl
> ---
>  drivers/mtd/mtdcore.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 2466ea466466..9b90e1be43e7 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1241,14 +1241,15 @@ int __get_mtd_device(struct mtd_info *mtd)
>  		return -ENODEV;
>  	}
> 
> -	kref_get(&mtd->refcnt);
> -
> -	while (mtd->parent) {
> -		if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ||
> mtd->parent != master)
> -			kref_get(&mtd->parent->refcnt);
> +	while (mtd) {
> +		if (mtd != master)
> +			kref_get(&mtd->refcnt);
>  		mtd = mtd->parent;
>  	}
> 
> +	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> +		kref_get(&master->refcnt);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(__get_mtd_device);
> @@ -1332,11 +1333,10 @@ void __put_mtd_device(struct mtd_info *mtd)
>  {
>  	struct mtd_info *master = mtd_get_master(mtd);
> 
> -	while (mtd != master) {
> -		struct mtd_info *parent = mtd->parent;
> -
> -		kref_put(&mtd->refcnt, mtd_device_release);
> -		mtd = parent;
> +	while (mtd) {
> +		if (mtd != master)
> +			kref_put(&mtd->refcnt, mtd_device_release);
kref_put may release mtd here, so the mtd->parent should be stored as in the original patch.

> +		mtd = mtd->parent;
>  	}
> 
>  	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> --
> 2.34.1


-- 
Thanks,
Sasha