| Message ID | YHRj5MUb5MGdPQqw@makrotopia.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | [v3] mtd: super: don't rely on mtdblock device minor | expand |
Hi Daniel, Daniel Golle <daniel@makrotopia.org> wrote on Mon, 12 Apr 2021 16:14:44 +0100: > For blktrans devices with partitions (ie. part_bits != 0) the > assumption that the minor number of the mtdblock device matches > the mtdnum doesn't hold true. > Properly resolve mtd device from blktrans layer instead. Unfortunately I cannot apply your patch anymore, would you mind rebasing it on top of v5.13-rc1? Thanks, Miquèl > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > v3: fix typo in patch description > v2: remove BUG() calls as requested by Miquel Raynal > > drivers/mtd/mtdsuper.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c > index c3e2098372f2e..d3af80a7ce86e 100644 > --- a/drivers/mtd/mtdsuper.c > +++ b/drivers/mtd/mtdsuper.c > @@ -9,6 +9,7 @@ > */ > > #include <linux/mtd/super.h> > +#include <linux/mtd/blktrans.h> > #include <linux/namei.h> > #include <linux/export.h> > #include <linux/ctype.h> > @@ -121,7 +122,8 @@ int get_tree_mtd(struct fs_context *fc, > { > #ifdef CONFIG_BLOCK > struct block_device *bdev; > - int ret, major; > + struct mtd_blktrans_dev *blktrans_dev; > + int ret, major, part_bits; > #endif > int mtdnr; > > @@ -169,21 +171,36 @@ int get_tree_mtd(struct fs_context *fc, > /* try the old way - the hack where we allowed users to mount > * /dev/mtdblock$(n) but didn't actually _use_ the blockdev > */ > - bdev = lookup_bdev(fc->source); > + bdev = blkdev_get_by_path(fc->source, FMODE_READ, NULL); > if (IS_ERR(bdev)) { > ret = PTR_ERR(bdev); > errorf(fc, "MTD: Couldn't look up '%s': %d", fc->source, ret); > return ret; > } > - pr_debug("MTDSB: lookup_bdev() returned 0\n"); > + pr_debug("MTDSB: blkdev_get_by_path() returned 0\n"); > > major = MAJOR(bdev->bd_dev); > - mtdnr = MINOR(bdev->bd_dev); > - bdput(bdev); > > - if (major == MTD_BLOCK_MAJOR) > - return mtd_get_sb_by_nr(fc, mtdnr, fill_super); > + if (major == MTD_BLOCK_MAJOR) { > + if (!bdev->bd_disk) { > + blkdev_put(bdev, FMODE_READ); > + return -EINVAL; > + } > + > + blktrans_dev = (struct mtd_blktrans_dev *)(bdev->bd_disk->private_data); > + if (!blktrans_dev || !blktrans_dev->tr) { > + blkdev_put(bdev, FMODE_READ); > + return -EINVAL; > + } > + mtdnr = blktrans_dev->devnum; > + part_bits = blktrans_dev->tr->part_bits; > + blkdev_put(bdev, FMODE_READ); > + if (MINOR(bdev->bd_dev) != (mtdnr << part_bits)) > + return -EINVAL; > > + return mtd_get_sb_by_nr(fc, mtdnr, fill_super); > + } > + blkdev_put(bdev, FMODE_READ); > #endif /* CONFIG_BLOCK */ > > if (!(fc->sb_flags & SB_SILENT))
On Mon, May 10, 2021 at 12:18:37PM +0200, Miquel Raynal wrote: > Hi Daniel, > > Daniel Golle <daniel@makrotopia.org> wrote on Mon, 12 Apr 2021 16:14:44 > +0100: > > > For blktrans devices with partitions (ie. part_bits != 0) the > > assumption that the minor number of the mtdblock device matches > > the mtdnum doesn't hold true. > > Properly resolve mtd device from blktrans layer instead. > > Unfortunately I cannot apply your patch anymore, would you mind > rebasing it on top of v5.13-rc1? I will send a rebased and improved version. Thank you! > > Thanks, > Miquèl > > > > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > --- > > v3: fix typo in patch description > > v2: remove BUG() calls as requested by Miquel Raynal > > > > drivers/mtd/mtdsuper.c | 31 ++++++++++++++++++++++++------- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c > > index c3e2098372f2e..d3af80a7ce86e 100644 > > --- a/drivers/mtd/mtdsuper.c > > +++ b/drivers/mtd/mtdsuper.c > > @@ -9,6 +9,7 @@ > > */ > > > > #include <linux/mtd/super.h> > > +#include <linux/mtd/blktrans.h> > > #include <linux/namei.h> > > #include <linux/export.h> > > #include <linux/ctype.h> > > @@ -121,7 +122,8 @@ int get_tree_mtd(struct fs_context *fc, > > { > > #ifdef CONFIG_BLOCK > > struct block_device *bdev; > > - int ret, major; > > + struct mtd_blktrans_dev *blktrans_dev; > > + int ret, major, part_bits; > > #endif > > int mtdnr; > > > > @@ -169,21 +171,36 @@ int get_tree_mtd(struct fs_context *fc, > > /* try the old way - the hack where we allowed users to mount > > * /dev/mtdblock$(n) but didn't actually _use_ the blockdev > > */ > > - bdev = lookup_bdev(fc->source); > > + bdev = blkdev_get_by_path(fc->source, FMODE_READ, NULL); > > if (IS_ERR(bdev)) { > > ret = PTR_ERR(bdev); > > errorf(fc, "MTD: Couldn't look up '%s': %d", fc->source, ret); > > return ret; > > } > > - pr_debug("MTDSB: lookup_bdev() returned 0\n"); > > + pr_debug("MTDSB: blkdev_get_by_path() returned 0\n"); > > > > major = MAJOR(bdev->bd_dev); > > - mtdnr = MINOR(bdev->bd_dev); > > - bdput(bdev); > > > > - if (major == MTD_BLOCK_MAJOR) > > - return mtd_get_sb_by_nr(fc, mtdnr, fill_super); > > + if (major == MTD_BLOCK_MAJOR) { > > + if (!bdev->bd_disk) { > > + blkdev_put(bdev, FMODE_READ); > > + return -EINVAL; > > + } > > + > > + blktrans_dev = (struct mtd_blktrans_dev *)(bdev->bd_disk->private_data); > > + if (!blktrans_dev || !blktrans_dev->tr) { > > + blkdev_put(bdev, FMODE_READ); > > + return -EINVAL; > > + } > > + mtdnr = blktrans_dev->devnum; > > + part_bits = blktrans_dev->tr->part_bits; > > + blkdev_put(bdev, FMODE_READ); > > + if (MINOR(bdev->bd_dev) != (mtdnr << part_bits)) > > + return -EINVAL; > > > > + return mtd_get_sb_by_nr(fc, mtdnr, fill_super); > > + } > > + blkdev_put(bdev, FMODE_READ); > > #endif /* CONFIG_BLOCK */ > > > > if (!(fc->sb_flags & SB_SILENT)) >
diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c index c3e2098372f2e..d3af80a7ce86e 100644 --- a/drivers/mtd/mtdsuper.c +++ b/drivers/mtd/mtdsuper.c @@ -9,6 +9,7 @@ */ #include <linux/mtd/super.h> +#include <linux/mtd/blktrans.h> #include <linux/namei.h> #include <linux/export.h> #include <linux/ctype.h> @@ -121,7 +122,8 @@ int get_tree_mtd(struct fs_context *fc, { #ifdef CONFIG_BLOCK struct block_device *bdev; - int ret, major; + struct mtd_blktrans_dev *blktrans_dev; + int ret, major, part_bits; #endif int mtdnr; @@ -169,21 +171,36 @@ int get_tree_mtd(struct fs_context *fc, /* try the old way - the hack where we allowed users to mount * /dev/mtdblock$(n) but didn't actually _use_ the blockdev */ - bdev = lookup_bdev(fc->source); + bdev = blkdev_get_by_path(fc->source, FMODE_READ, NULL); if (IS_ERR(bdev)) { ret = PTR_ERR(bdev); errorf(fc, "MTD: Couldn't look up '%s': %d", fc->source, ret); return ret; } - pr_debug("MTDSB: lookup_bdev() returned 0\n"); + pr_debug("MTDSB: blkdev_get_by_path() returned 0\n"); major = MAJOR(bdev->bd_dev); - mtdnr = MINOR(bdev->bd_dev); - bdput(bdev); - if (major == MTD_BLOCK_MAJOR) - return mtd_get_sb_by_nr(fc, mtdnr, fill_super); + if (major == MTD_BLOCK_MAJOR) { + if (!bdev->bd_disk) { + blkdev_put(bdev, FMODE_READ); + return -EINVAL; + } + + blktrans_dev = (struct mtd_blktrans_dev *)(bdev->bd_disk->private_data); + if (!blktrans_dev || !blktrans_dev->tr) { + blkdev_put(bdev, FMODE_READ); + return -EINVAL; + } + mtdnr = blktrans_dev->devnum; + part_bits = blktrans_dev->tr->part_bits; + blkdev_put(bdev, FMODE_READ); + if (MINOR(bdev->bd_dev) != (mtdnr << part_bits)) + return -EINVAL; + return mtd_get_sb_by_nr(fc, mtdnr, fill_super); + } + blkdev_put(bdev, FMODE_READ); #endif /* CONFIG_BLOCK */ if (!(fc->sb_flags & SB_SILENT))
For blktrans devices with partitions (ie. part_bits != 0) the assumption that the minor number of the mtdblock device matches the mtdnum doesn't hold true. Properly resolve mtd device from blktrans layer instead. Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- v3: fix typo in patch description v2: remove BUG() calls as requested by Miquel Raynal drivers/mtd/mtdsuper.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)