[PATCH 2/2] mtd: key superblock by device number

[Jan Kara]

On Tue 29-08-23 17:23:57, Christian Brauner wrote:
> The mtd driver has similar problems than the one that was fixed in
> commit dc3216b14160 ("super: ensure valid info").
> 
> The kill_mtd_super() helper calls shuts the superblock down but leaves
> the superblock on fs_supers as the devices are still in use but puts the
> mtd device and cleans out the superblock's s_mtd field.
> 
> This means another mounter can find the superblock on the list accessing
> its s_mtd field while it is curently in the process of being freed or
> already freed.
> 
> Prevent that from happening by keying superblock by dev_t just as we do
> in the generic code.
> 
> Link: https://lore.kernel.org/linux-fsdevel/20230829-weitab-lauwarm-49c40fc85863@brauner
> Signed-off-by: Christian Brauner <brauner at kernel.org>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack at suse.cz>

								Honza

> ---
>  drivers/mtd/mtdsuper.c | 45 +++++++++++----------------------------------
>  1 file changed, 11 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
> index 5ff001140ef4..b7e3763c47f0 100644
> --- a/drivers/mtd/mtdsuper.c
> +++ b/drivers/mtd/mtdsuper.c
> @@ -19,38 +19,6 @@
>  #include <linux/fs_context.h>
>  #include "mtdcore.h"
>  
> -/*
> - * compare superblocks to see if they're equivalent
> - * - they are if the underlying MTD device is the same
> - */
> -static int mtd_test_super(struct super_block *sb, struct fs_context *fc)
> -{
> -	struct mtd_info *mtd = fc->sget_key;
> -
> -	if (sb->s_mtd == fc->sget_key) {
> -		pr_debug("MTDSB: Match on device %d (\"%s\")\n",
> -			 mtd->index, mtd->name);
> -		return 1;
> -	}
> -
> -	pr_debug("MTDSB: No match, device %d (\"%s\"), device %d (\"%s\")\n",
> -		 sb->s_mtd->index, sb->s_mtd->name, mtd->index, mtd->name);
> -	return 0;
> -}
> -
> -/*
> - * mark the superblock by the MTD device it is using
> - * - set the device number to be the correct MTD block device for pesuperstence
> - *   of NFS exports
> - */
> -static int mtd_set_super(struct super_block *sb, struct fs_context *fc)
> -{
> -	sb->s_mtd = fc->sget_key;
> -	sb->s_dev = MKDEV(MTD_BLOCK_MAJOR, sb->s_mtd->index);
> -	sb->s_bdi = bdi_get(mtd_bdi);
> -	return 0;
> -}
> -
>  /*
>   * get a superblock on an MTD-backed filesystem
>   */
> @@ -62,8 +30,7 @@ static int mtd_get_sb(struct fs_context *fc,
>  	struct super_block *sb;
>  	int ret;
>  
> -	fc->sget_key = mtd;
> -	sb = sget_fc(fc, mtd_test_super, mtd_set_super);
> +	sb = sget_dev(fc, MKDEV(MTD_BLOCK_MAJOR, mtd->index));
>  	if (IS_ERR(sb))
>  		return PTR_ERR(sb);
>  
> @@ -77,6 +44,16 @@ static int mtd_get_sb(struct fs_context *fc,
>  		pr_debug("MTDSB: New superblock for device %d (\"%s\")\n",
>  			 mtd->index, mtd->name);
>  
> +		/*
> +		 * Would usually have been set with @sb_lock held but in
> +		 * contrast to sb->s_bdev that's checked with only
> +		 * @sb_lock held, nothing checks sb->s_mtd without also
> +		 * holding sb->s_umount and we're holding sb->s_umount
> +		 * here.
> +		 */
> +		sb->s_mtd = mtd;
> +		sb->s_bdi = bdi_get(mtd_bdi);
> +
>  		ret = fill_super(sb, fc);
>  		if (ret < 0)
>  			goto error_sb;
> 
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack at suse.com>
SUSE Labs, CR