2019-09-26 14:25:28

by David Howells

[permalink] [raw]
Subject: [PATCH] jffs2: Fix mounting under new mount API

The mounting of jffs2 is broken due to the changes from the new mount API
because it specifies a "source" operation, but then doesn't actually
process it. But because it specified it, it doesn't return -ENOPARAM and
the caller doesn't process it either and the source gets lost.

Fix this by simply removing the source parameter from jffs2 and letting the
VFS deal with it in the default manner.

To test it, enable CONFIG_MTD_MTDRAM and allow the default size and erase
block size parameters, then try and mount the /dev/mtdblock<N> file that
that creates as jffs2. No need to initialise it.

Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
Reported-by: Al Viro <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: David Woodhouse <[email protected]>
cc: Richard Weinberger <[email protected]>
cc: [email protected]
---

fs/jffs2/super.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index cbe70637c117..0e6406c4f362 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -163,13 +163,11 @@ static const struct export_operations jffs2_export_ops = {
* Opt_rp_size: size of reserved pool in KiB
*/
enum {
- Opt_source,
Opt_override_compr,
Opt_rp_size,
};

static const struct fs_parameter_spec jffs2_param_specs[] = {
- fsparam_string ("source", Opt_source),
fsparam_enum ("compr", Opt_override_compr),
fsparam_u32 ("rp_size", Opt_rp_size),
{}


2019-09-26 14:28:41

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] jffs2: Fix mounting under new mount API

On Thu, Sep 26, 2019 at 03:21:18PM +0100, David Howells wrote:
> The mounting of jffs2 is broken due to the changes from the new mount API
> because it specifies a "source" operation, but then doesn't actually
> process it. But because it specified it, it doesn't return -ENOPARAM and
> the caller doesn't process it either and the source gets lost.
>
> Fix this by simply removing the source parameter from jffs2 and letting the
> VFS deal with it in the default manner.
>
> To test it, enable CONFIG_MTD_MTDRAM and allow the default size and erase
> block size parameters, then try and mount the /dev/mtdblock<N> file that
> that creates as jffs2. No need to initialise it.
>
> Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
> Reported-by: Al Viro <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: David Woodhouse <[email protected]>
> cc: Richard Weinberger <[email protected]>
> cc: [email protected]

Applied.

2019-09-27 08:39:35

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] jffs2: Fix mounting under new mount API

Hello!

On 26.09.2019 17:21, David Howells wrote:

> The mounting of jffs2 is broken due to the changes from the new mount API
> because it specifies a "source" operation, but then doesn't actually
> process it. But because it specified it, it doesn't return -ENOPARAM and

What specified what? Too many "it"'s to figure that out. :-)

> the caller doesn't process it either and the source gets lost.
>
> Fix this by simply removing the source parameter from jffs2 and letting the
> VFS deal with it in the default manner.
>
> To test it, enable CONFIG_MTD_MTDRAM and allow the default size and erase
> block size parameters, then try and mount the /dev/mtdblock<N> file that
> that creates as jffs2. No need to initialise it.

One "that" should be enough. :-)

> Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
> Reported-by: Al Viro <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: David Woodhouse <[email protected]>
> cc: Richard Weinberger <[email protected]>
> cc: [email protected]
[...]

MBR, Sergei

2019-11-13 20:39:59

by Han Xu

[permalink] [raw]
Subject: Re: [PATCH] jffs2: Fix mounting under new mount API

Tested the JFFS2 on 5.4 kernel as the instruction said, still got some
errors, any ideas?

Without the patch,

root@imx8mmevk:~# cat /proc/mtd
dev: size erasesize name
mtd0: 00400000 00020000 "mtdram test device"
mtd1: 04000000 00020000 "5d120000.spi"
root@imx8mmevk:~# mtd_debug info /dev/mtd0
mtd.type = MTD_RAM
mtd.flags = MTD_CAP_RAM
mtd.size = 4194304 (4M)
mtd.erasesize = 131072 (128K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0

root@imx8mmevk:~# flash_erase /dev/mtd0 0 0
Erasing 128 Kibyte @ 3e0000 -- 100 % complete
root@imx8mmevk:~# mount -t jffs2 /dev/mtdblock0 test_dir/
mount: /home/root/test_dir: wrong fs type, bad option, bad superblock
on /dev/mtdblock0, missing codepage or helper program, or other error.

With the patch,

root@imx8mmevk:~# cat /proc/mtd
dev: size erasesize name
mtd0: 00400000 00020000 "mtdram test device"
mtd1: 04000000 00020000 "5d120000.spi"
root@imx8mmevk:~# mtd_debug info /dev/mtd0
mtd.type = MTD_RAM
mtd.flags = MTD_CAP_RAM
mtd.size = 4194304 (4M)
mtd.erasesize = 131072 (128K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0

root@imx8mmevk:~# flash_erase /dev/mtd0 0 0
Erasing 128 Kibyte @ 3e0000 -- 100 % complete
root@imx8mmevk:~# mount -t jffs2 /dev/mtdblock0 test_dir/
root@imx8mmevk:~# mount
/dev/mtdblock0 on /home/root/test_dir type jffs2 (rw,relatime)

BUT, it's not writable.

root@imx8mmevk:~# cp test_file test_dir/
cp: error writing 'test_dir/test_file': Invalid argument

root@imx8mmevk:~# dd if=/dev/urandom of=test_dir/test_file bs=1k count=1
dd: error writing 'test_dir/test_file': Invalid argument
1+0 records in
0+0 records out
0 bytes copied, 0.000855156 s, 0.0 kB/s


On Fri, Sep 27, 2019 at 3:38 AM Sergei Shtylyov
<[email protected]> wrote:
>
> Hello!
>
> On 26.09.2019 17:21, David Howells wrote:
>
> > The mounting of jffs2 is broken due to the changes from the new mount API
> > because it specifies a "source" operation, but then doesn't actually
> > process it. But because it specified it, it doesn't return -ENOPARAM and
>
> What specified what? Too many "it"'s to figure that out. :-)
>
> > the caller doesn't process it either and the source gets lost.
> >
> > Fix this by simply removing the source parameter from jffs2 and letting the
> > VFS deal with it in the default manner.
> >
> > To test it, enable CONFIG_MTD_MTDRAM and allow the default size and erase
> > block size parameters, then try and mount the /dev/mtdblock<N> file that
> > that creates as jffs2. No need to initialise it.
>
> One "that" should be enough. :-)
>
> > Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
> > Reported-by: Al Viro <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > cc: David Woodhouse <[email protected]>
> > cc: Richard Weinberger <[email protected]>
> > cc: [email protected]
> [...]
>
> MBR, Sergei
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



--
Sincerely,

Han XU

2019-11-14 12:04:01

by Hou Tao

[permalink] [raw]
Subject: Re: [PATCH] jffs2: Fix mounting under new mount API

Hi,

On 2019/11/14 4:38, Han Xu wrote:
> Tested the JFFS2 on 5.4 kernel as the instruction said, still got some
> errors, any ideas?
>

>
> With the patch,
>
> root@imx8mmevk:~# cat /proc/mtd
> dev: size erasesize name
> mtd0: 00400000 00020000 "mtdram test device"
> mtd1: 04000000 00020000 "5d120000.spi"
> root@imx8mmevk:~# mtd_debug info /dev/mtd0
> mtd.type = MTD_RAM
> mtd.flags = MTD_CAP_RAM
> mtd.size = 4194304 (4M)
> mtd.erasesize = 131072 (128K)
> mtd.writesize = 1
> mtd.oobsize = 0
> regions = 0
>
> root@imx8mmevk:~# flash_erase /dev/mtd0 0 0
> Erasing 128 Kibyte @ 3e0000 -- 100 % complete
> root@imx8mmevk:~# mount -t jffs2 /dev/mtdblock0 test_dir/
> root@imx8mmevk:~# mount
> /dev/mtdblock0 on /home/root/test_dir type jffs2 (rw,relatime)
>
> BUT, it's not writable.

You should revert the following commit to make it work:

commit f2538f999345405f7d2e1194c0c8efa4e11f7b3a
Author: Jia-Ju Bai <[email protected]>
Date: Wed Jul 24 10:46:58 2019 +0800

jffs2: Fix possible null-pointer dereferences in jffs2_add_frag_to_fragtree()

The revert needs to get into v5.4. Maybe Richard has forget about it ?

Regards,
Tao

>
> root@imx8mmevk:~# cp test_file test_dir/
> cp: error writing 'test_dir/test_file': Invalid argument
>
> root@imx8mmevk:~# dd if=/dev/urandom of=test_dir/test_file bs=1k count=1
> dd: error writing 'test_dir/test_file': Invalid argument
> 1+0 records in
> 0+0 records out
> 0 bytes copied, 0.000855156 s, 0.0 kB/s
>
>
> On Fri, Sep 27, 2019 at 3:38 AM Sergei Shtylyov
> <[email protected]> wrote:
>>
>> Hello!
>>
>> On 26.09.2019 17:21, David Howells wrote:
>>
>>> The mounting of jffs2 is broken due to the changes from the new mount API
>>> because it specifies a "source" operation, but then doesn't actually
>>> process it. But because it specified it, it doesn't return -ENOPARAM and
>>
>> What specified what? Too many "it"'s to figure that out. :-)
>>
>>> the caller doesn't process it either and the source gets lost.
>>>
>>> Fix this by simply removing the source parameter from jffs2 and letting the
>>> VFS deal with it in the default manner.
>>>
>>> To test it, enable CONFIG_MTD_MTDRAM and allow the default size and erase
>>> block size parameters, then try and mount the /dev/mtdblock<N> file that
>>> that creates as jffs2. No need to initialise it.
>>
>> One "that" should be enough. :-)
>>
>>> Fixes: ec10a24f10c8 ("vfs: Convert jffs2 to use the new mount API")
>>> Reported-by: Al Viro <[email protected]>
>>> Signed-off-by: David Howells <[email protected]>
>>> cc: David Woodhouse <[email protected]>
>>> cc: Richard Weinberger <[email protected]>
>>> cc: [email protected]
>> [...]
>>
>> MBR, Sergei
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
>

2019-11-29 00:01:23

by Joel Stanley

[permalink] [raw]
Subject: Re: [PATCH] jffs2: Fix mounting under new mount API

On Thu, 14 Nov 2019 at 12:05, Hou Tao <[email protected]> wrote:
>
> Hi,
>
> On 2019/11/14 4:38, Han Xu wrote:
> > Tested the JFFS2 on 5.4 kernel as the instruction said, still got some
> > errors, any ideas?
> >
>
> >
> > With the patch,
> >
> > root@imx8mmevk:~# cat /proc/mtd
> > dev: size erasesize name
> > mtd0: 00400000 00020000 "mtdram test device"
> > mtd1: 04000000 00020000 "5d120000.spi"
> > root@imx8mmevk:~# mtd_debug info /dev/mtd0
> > mtd.type = MTD_RAM
> > mtd.flags = MTD_CAP_RAM
> > mtd.size = 4194304 (4M)
> > mtd.erasesize = 131072 (128K)
> > mtd.writesize = 1
> > mtd.oobsize = 0
> > regions = 0
> >
> > root@imx8mmevk:~# flash_erase /dev/mtd0 0 0
> > Erasing 128 Kibyte @ 3e0000 -- 100 % complete
> > root@imx8mmevk:~# mount -t jffs2 /dev/mtdblock0 test_dir/
> > root@imx8mmevk:~# mount
> > /dev/mtdblock0 on /home/root/test_dir type jffs2 (rw,relatime)
> >
> > BUT, it's not writable.
>
> You should revert the following commit to make it work:
>
> commit f2538f999345405f7d2e1194c0c8efa4e11f7b3a
> Author: Jia-Ju Bai <[email protected]>
> Date: Wed Jul 24 10:46:58 2019 +0800
>
> jffs2: Fix possible null-pointer dereferences in jffs2_add_frag_to_fragtree()
>
> The revert needs to get into v5.4. Maybe Richard has forget about it ?

I hit this today. The error I saw was:

[ 4.975868] jffs2: error: (77) jffs2_build_inode_fragtree: Add node
to tree failed -22
[ 4.983923] jffs2: error: (77) jffs2_do_read_inode_internal: Failed
to build final fragtree for inode #5377: error -22
[ 4.994709] jffs2: Returned error for crccheck of ino #5377. Expect
badness...

Reverting the f2538f999345 commit fixed things.

Is the revert queued for stable?

Cheers,

Joel