mtd

mtd

[Amit.Lubovsky]

Hi,
I did as it is said in document
mtd-jffs-HOWTO.txt, v 1.13 2001/05/01 to add MTD support in linux kernel 2.4 for my mips 5kc on my malta board.
I have 2 pcs. flash chips on the board (Intel DT28F160) which has 4 MB capacity.
So I configured xconfig with CFI support, i.e:
CONFIG_MTD
CONFIG_MTD_ROM
CONFIG_MTD_CFI
CONFIG_MTD_CFI_ADVANCED_OPTIONS
CONFIG_MTD_CFI_NOSWAP
CONFIG_MTD_CFI_GEOMETRY
CONFIG_MTD_CFI_B4
CONFIG_MTD_CFI_I4
CONFIG_MTD_CFI_INTELEXT
CONFIG_MTD_CFI_PHYSMAP
CONFIG_MTD_CFI_PHYSMAP_START=1E000000
CONFIG_MTD_CFI_LEN=390900
CONFIG_MTD_CFI_BUSWIDTH=4

since it didn't worked I have change the function ioremap() in physmap.c to ioremap_nocache() but it still doesn't work...
Any clue???

Thanks, Amit.

Re: mtd

[David Woodhouse]

Turn off CONFIG_MTD_CFI_ADVANCED_OPTIONS.

--
dwmw2

mtd

[Christian Brauner]

I just looked through every single kill_sb once more with an eye
specifically on the bug we just fixed. While doing so I realized that
mtd devices are borked. Taking jffs2 as an example we have:

static void jffs2_kill_sb(struct super_block *sb)
{
        struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
        if (c && !sb_rdonly(sb))
                jffs2_stop_garbage_collect_thread(c);
        kill_mtd_super(sb);
        kfree(c);
}

kill_mtd_super() calls generic_shutdown_super() which shuts the sb down
but leaves the superblock on fs_supers - which is what we want as the
devices are still in use. But then afterwards it puts the mtd device and
cleans out sb->s_mtd:

void kill_mtd_super(struct super_block *sb)
{
        generic_shutdown_super(sb);
        put_mtd_device(sb->s_mtd);
        sb->s_mtd = NULL;
}

But as you can see in

static int mtd_get_sb()
{
         fc->sget_key = mtd;
         sb = sget_fc(fc, mtd_test_super, mtd_set_super);
}

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;
}

it can UAF if s_mtd is freed during put_mtd_device(). Yes, there's also
a data race but that's not that problematic.

Of course, the simple hotfix is to notify from kill_mtd_super() and
fixup cramfs and romfs but the proper fix is to do what we did for
get_tree_bdev() and friends and key mtd devices by dev_t. The patch
should be fairly small, I think.

Anyone has cycles to tackle this or should I try?

Something like the following might already be enough (IT'S A DRAFT, AND
UNTESTED, AND PROBABLY BROKEN)?

diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index 5ff001140ef4..992a65d4b90b 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -25,16 +25,15 @@
  */
 static int mtd_test_super(struct super_block *sb, struct fs_context *fc)
 {
-       struct mtd_info *mtd = fc->sget_key;
+       dev_t dev = *(dev_t *)fc->sget_key;

-       if (sb->s_mtd == fc->sget_key) {
-               pr_debug("MTDSB: Match on device %d (\"%s\")\n",
-                        mtd->index, mtd->name);
+       if (sb->s_dev == dev) {
+               pr_debug("MTDSB: Match on device %d\n", MINOR(sb->s_dev));
                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);
+       pr_debug("MTDSB: No match, device %d, device %d\n",
+                MINOR(sb->s_dev), MINOR(dev));
        return 0;
 }

@@ -45,9 +44,7 @@ static int mtd_test_super(struct super_block *sb, struct fs_context *fc)
  */
 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;
 }

@@ -61,8 +58,9 @@ static int mtd_get_sb(struct fs_context *fc,
 {
        struct super_block *sb;
        int ret;
+       dev_t dev = MKDEV(MTD_BLOCK_MAJOR, mtd->index);

-       fc->sget_key = mtd;
+       fc->sget_key = &dev;
        sb = sget_fc(fc, mtd_test_super, mtd_set_super);
        if (IS_ERR(sb))
                return PTR_ERR(sb);
@@ -77,6 +75,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 in e.g.,
+                * get_active_super() with only @sb_lock held, nothing seems to
+                * check 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;

Re: mtd

[Christoph Hellwig]

On Tue, Aug 29, 2023 at 01:46:20PM +0200, Christian Brauner wrote:
> Something like the following might already be enough (IT'S A DRAFT, AND
> UNTESTED, AND PROBABLY BROKEN)?

It's probably the right thing conceptually, but it will also need
the SB_I_RETIRED from test_bdev_super_fc or even just reuse
test_bdev_super_fc after that's been renamed to be more generic.

In fact I've been wondering for a while why we even support the magic
keyed get_super - if it allocates a new super it should also have a
new dev_t.  So IMHO we should stop playing stupid tricks with keys and
just declare the dev_t the key after doing all the required work for it,
that is allocating the per-instance anon dev_t in the caller.

Re: mtd

[Christian Brauner]

On Tue, Aug 29, 2023 at 02:51:18PM +0200, Christoph Hellwig wrote:
> On Tue, Aug 29, 2023 at 01:46:20PM +0200, Christian Brauner wrote:
> > Something like the following might already be enough (IT'S A DRAFT, AND
> > UNTESTED, AND PROBABLY BROKEN)?
> 
> It's probably the right thing conceptually, but it will also need
> the SB_I_RETIRED from test_bdev_super_fc or even just reuse
> test_bdev_super_fc after that's been renamed to be more generic.

I'll rename it and use it. Let me send a patch.

Re: mtd

[Christian Brauner]

On Tue, Aug 29, 2023 at 02:57:02PM +0200, Christian Brauner wrote:
> On Tue, Aug 29, 2023 at 02:51:18PM +0200, Christoph Hellwig wrote:
> > On Tue, Aug 29, 2023 at 01:46:20PM +0200, Christian Brauner wrote:
> > > Something like the following might already be enough (IT'S A DRAFT, AND
> > > UNTESTED, AND PROBABLY BROKEN)?
> > 
> > It's probably the right thing conceptually, but it will also need
> > the SB_I_RETIRED from test_bdev_super_fc or even just reuse
> > test_bdev_super_fc after that's been renamed to be more generic.
> 
> I'll rename it and use it. Let me send a patch.

Hmkay, how does that look? I think this is a fairly acceptable change
and looks better than the mtd special-test/set-sauce we currently have:

From b85ee296f59b0a8e739f10ab9005b7c1fe1aad23 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 29 Aug 2023 15:05:28 +0200
Subject: [PATCH 1/2] fs: export vfs_super_s_dev_{set,test} helpers

They will be used in other places as well.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/super.c         | 8 +++++---
 include/linux/fs.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index ad7ac3a24d38..a122154facbf 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1435,16 +1435,18 @@ static int set_bdev_super(struct super_block *s, void *data)
 	return 0;
 }
 
-static int set_bdev_super_fc(struct super_block *s, struct fs_context *fc)
+int vfs_super_s_dev_set(struct super_block *s, struct fs_context *fc)
 {
 	return set_bdev_super(s, fc->sget_key);
 }
+EXPORT_SYMBOL(vfs_super_s_dev_set);
 
-static int test_bdev_super_fc(struct super_block *s, struct fs_context *fc)
+int vfs_super_s_dev_test(struct super_block *s, struct fs_context *fc)
 {
 	return !(s->s_iflags & SB_I_RETIRED) &&
 		s->s_dev == *(dev_t *)fc->sget_key;
 }
+EXPORT_SYMBOL(vfs_super_s_dev_test);
 
 int setup_bdev_super(struct super_block *sb, int sb_flags,
 		struct fs_context *fc)
@@ -1524,7 +1526,7 @@ int get_tree_bdev(struct fs_context *fc,
 
 	fc->sb_flags |= SB_NOSEC;
 	fc->sget_key = &dev;
-	s = sget_fc(fc, test_bdev_super_fc, set_bdev_super_fc);
+	s = sget_fc(fc, vfs_super_s_dev_set, vfs_super_s_dev_test);
 	if (IS_ERR(s))
 		return PTR_ERR(s);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ca8ceccde3d6..fd32ae238700 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2274,6 +2274,8 @@ struct super_block *sget(struct file_system_type *type,
 			int (*test)(struct super_block *,void *),
 			int (*set)(struct super_block *,void *),
 			int flags, void *data);
+int vfs_super_s_dev_set(struct super_block *s, struct fs_context *fc);
+int vfs_super_s_dev_test(struct super_block *s, struct fs_context *fc);
 
 /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
 #define fops_get(fops) \
-- 
2.34.1

From a91589157e4582182d48a5b7451c4303add26a69 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Tue, 29 Aug 2023 14:58:33 +0200
Subject: [PATCH 2/2] mtd: key superblock by device number

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@kernel.org>
---
 drivers/mtd/mtdsuper.c | 47 ++++++++++++------------------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index 5ff001140ef4..29870a375743 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
  */
@@ -61,9 +29,10 @@ static int mtd_get_sb(struct fs_context *fc,
 {
 	struct super_block *sb;
 	int ret;
+	dev_t dev = MKDEV(MTD_BLOCK_MAJOR, mtd->index);
 
-	fc->sget_key = mtd;
-	sb = sget_fc(fc, mtd_test_super, mtd_set_super);
+	fc->sget_key = &dev;
+	sb = sget_fc(fc, vfs_super_s_dev_test, vfs_super_s_dev_set);
 	if (IS_ERR(sb))
 		return PTR_ERR(sb);
 
@@ -77,6 +46,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

Re: mtd

[Christoph Hellwig]

On Tue, Aug 29, 2023 at 03:41:04PM +0200, Christian Brauner wrote:
> On Tue, Aug 29, 2023 at 02:57:02PM +0200, Christian Brauner wrote:
> > On Tue, Aug 29, 2023 at 02:51:18PM +0200, Christoph Hellwig wrote:
> > > On Tue, Aug 29, 2023 at 01:46:20PM +0200, Christian Brauner wrote:
> > > > Something like the following might already be enough (IT'S A DRAFT, AND
> > > > UNTESTED, AND PROBABLY BROKEN)?
> > > 
> > > It's probably the right thing conceptually, but it will also need
> > > the SB_I_RETIRED from test_bdev_super_fc or even just reuse
> > > test_bdev_super_fc after that's been renamed to be more generic.
> > 
> > I'll rename it and use it. Let me send a patch.
> 
> Hmkay, how does that look? I think this is a fairly acceptable change
> and looks better than the mtd special-test/set-sauce we currently have:

Looks sensibe to me, but please run it past the MTD maintainers.

Re: mtd

[Christian Brauner]

On Tue, Aug 29, 2023 at 04:09:53PM +0200, Christoph Hellwig wrote:
> On Tue, Aug 29, 2023 at 03:41:04PM +0200, Christian Brauner wrote:
> > On Tue, Aug 29, 2023 at 02:57:02PM +0200, Christian Brauner wrote:
> > > On Tue, Aug 29, 2023 at 02:51:18PM +0200, Christoph Hellwig wrote:
> > > > On Tue, Aug 29, 2023 at 01:46:20PM +0200, Christian Brauner wrote:
> > > > > Something like the following might already be enough (IT'S A DRAFT, AND
> > > > > UNTESTED, AND PROBABLY BROKEN)?
> > > > 
> > > > It's probably the right thing conceptually, but it will also need
> > > > the SB_I_RETIRED from test_bdev_super_fc or even just reuse
> > > > test_bdev_super_fc after that's been renamed to be more generic.
> > > 
> > > I'll rename it and use it. Let me send a patch.
> > 
> > Hmkay, how does that look? I think this is a fairly acceptable change
> > and looks better than the mtd special-test/set-sauce we currently have:
> 
> Looks sensibe to me, but please run it past the MTD maintainers.

Done.

mtd

[Levente]

Dear all,


I don't know if I post this to the right place, so forgive my ignorance.

My college discovered a possible bug in OpenWRT's mtd utility. When
writing to jffs device, the 'mtd' utility does not skip bad blocks.

This patch seems to fix this issue.


diff --git a/package/system/mtd/src/jffs2.c b/package/system/mtd/src/jffs2.c
index b432f64ac0..5bf3eec328 100644
--- a/package/system/mtd/src/jffs2.c
+++ b/package/system/mtd/src/jffs2.c
@@ -308,6 +308,16 @@ int mtd_write_jffs2(const char *mtd, const char
*filename, const char *dir)
        for(;;) {
                struct jffs2_unknown_node *node = (struct
jffs2_unknown_node *) buf;

+               while (mtd_block_is_bad(outfd, mtdofs) && (mtdofs < mtdsize)) {
+                       if (!quiet)
+                               fprintf(stderr, "\nSkipping bad block
at 0x%08x   ", mtdofs);
+
+                       mtdofs += erasesize;
+
+                       /* Move the file pointer along over the bad block. */
+                       lseek(outfd, erasesize, SEEK_CUR);
+               }
+
                if (read(outfd, buf, erasesize) != erasesize) {
                        fdeof = 1;
                        break;


I've cloned your utility repository, but didn't find jffs2.c anywhere.

Best regards,
Levente

Re: mtd

[David Oberhollenzer]

Hi,


I took a look at the OpenWRT source repository and the tool you mention looks more
like some home grown utility to me.

The mtd-utils you've cloned are built by package/utils/mtd-utils/Makefile

You may want to sort this out on the OpenWRT mailing list, unless of course the source
of that utility has been copied from mkfs.jffs2 at some point and the later happens
to have the same issue.

See: https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Thanks,

David

Re: mtd

[Levente]

I did this, and the OpenWRT guys directed me here.

But thank you for your help anyways.

Levente

On Fri, May 18, 2018 at 11:09 AM, David Oberhollenzer
<goliath@sigma-star.at> wrote:
> Hi,
>
>
> I took a look at the OpenWRT source repository and the tool you mention looks more
> like some home grown utility to me.
>
> The mtd-utils you've cloned are built by package/utils/mtd-utils/Makefile
>
> You may want to sort this out on the OpenWRT mailing list, unless of course the source
> of that utility has been copied from mkfs.jffs2 at some point and the later happens
> to have the same issue.
>
> See: https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
>
> Thanks,
>
> David

MTD

[Aneesh]

Hello,
        I am Using an arm9 processor(kernel 2.6.24) and tried to
implement JFFS2 in parallel flash for data storage.but it failed .
my parallel flash is AT49BV642DT .First i erased the flash and mounted
to a drive .then i created a file there .After that i unmounted the
drive .and
 when i am trying to mount the jffs2 again  ,It is showing an error like
follows
"Magic bitmask 0x1985 not found at 0x007e0078:  0x6d63 instead  "
can you help me to solve this problem ?
                                                    Regards
                                                            Aneesh.

Re: MTD

[Andrey Yurovsky]

Please see the FAQ:
http://www.linux-mtd.infradead.org/faq/jffs2.html#L_magicnfound

On Tue, Mar 25, 2008 at 8:58 PM, Aneesh <aneesh_g@cms.com> wrote:
>
>
>  Hello,
>         I am Using an arm9 processor(kernel 2.6.24) and tried to
>  implement JFFS2 in parallel flash for data storage.but it failed .
>  my parallel flash is AT49BV642DT .First i erased the flash and mounted
>  to a drive .then i created a file there .After that i unmounted the
>  drive .and
>   when i am trying to mount the jffs2 again  ,It is showing an error like
>  follows
>  "Magic bitmask 0x1985 not found at 0x007e0078:  0x6d63 instead  "
>  can you help me to solve this problem ?
>                                                     Regards
>                                                             Aneesh.
>
>
>  ______________________________________________________
>  Linux MTD discussion mailing list
>  http://lists.infradead.org/mailman/listinfo/linux-mtd/
>



-- 
Andrey Yurovsky
cozybit Inc.
p 415 974 6770 x14
f 415 974 6771
e andrey@cozybit.com

Mtd

[yogesh shivabasappa]

Hi 

I'm new to mtd devices. I'm using intelp30 Nor flash memory chip
in one of my project . i am accessing this as a character device
I wanted to know how this deffers from the normal character
devices

thanks 

yogesh

________________________________________________
Get your own "800" number
Voicemail, fax, email, and a lot more
http://www.ureach.com/reg/tag

mtd

[Bob Canup]

 I would like to possibly add something to the "to do" list:

Make sure the code works with older kernels. Much development work on
embedded systems is done on the proven stable 2.0 and 2.2 kernels.
Embedded systems tend not to have bleeding edge software in them for
reliability reasons. That is part of the reason that Alan Cox still
turns out 2.0.x kernels.

Bob




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: mtd

[David Woodhouse]

rcanup@go2fax.com said:
>  Make sure the code works with older kernels. Much development work on
> embedded systems is done on the proven stable 2.0 and 2.2 kernels.
> Embedded systems tend not to have bleeding edge software in them for
> reliability reasons. That is part of the reason that Alan Cox still
> turns out 2.0.x kernels.

Agreed. Making it work with 2.2 is sort of implicit - I know Trevor's 
developing on 2.2, and I try to test with 2.2 periodically. Back-porting to 
2.0 could possibly be considered a separate task.

--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org