linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] jffs2: Fix mounting under new mount API
@ 2019-09-26 14:21 David Howells
  2019-09-26 14:26 ` Al Viro
  2019-09-27  8:38 ` Sergei Shtylyov
  0 siblings, 2 replies; 6+ messages in thread
From: David Howells @ 2019-09-26 14:21 UTC (permalink / raw)
  To: dwmw2, richard
  Cc: Al Viro, linux-mtd, dhowells, viro, linux-fsdevel, linux-kernel

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 <viro@zeniv.linux.org.uk>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: David Woodhouse <dwmw2@infradead.org>
cc: Richard Weinberger <richard@nod.at>
cc: linux-mtd@lists.infradead.org
---

 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),
 	{}


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] jffs2: Fix mounting under new mount API
  2019-09-26 14:21 [PATCH] jffs2: Fix mounting under new mount API David Howells
@ 2019-09-26 14:26 ` Al Viro
  2019-09-27  8:38 ` Sergei Shtylyov
  1 sibling, 0 replies; 6+ messages in thread
From: Al Viro @ 2019-09-26 14:26 UTC (permalink / raw)
  To: David Howells; +Cc: dwmw2, richard, linux-mtd, linux-fsdevel, linux-kernel

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 <viro@zeniv.linux.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: David Woodhouse <dwmw2@infradead.org>
> cc: Richard Weinberger <richard@nod.at>
> cc: linux-mtd@lists.infradead.org

Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] jffs2: Fix mounting under new mount API
  2019-09-26 14:21 [PATCH] jffs2: Fix mounting under new mount API David Howells
  2019-09-26 14:26 ` Al Viro
@ 2019-09-27  8:38 ` Sergei Shtylyov
  2019-11-13 20:38   ` Han Xu
  1 sibling, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2019-09-27  8:38 UTC (permalink / raw)
  To: David Howells, dwmw2, richard
  Cc: linux-fsdevel, linux-mtd, viro, linux-kernel

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 <viro@zeniv.linux.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: David Woodhouse <dwmw2@infradead.org>
> cc: Richard Weinberger <richard@nod.at>
> cc: linux-mtd@lists.infradead.org
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] jffs2: Fix mounting under new mount API
  2019-09-27  8:38 ` Sergei Shtylyov
@ 2019-11-13 20:38   ` Han Xu
  2019-11-14 12:01     ` Hou Tao
  0 siblings, 1 reply; 6+ messages in thread
From: Han Xu @ 2019-11-13 20:38 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David Howells, David Woodhouse, Richard Weinberger,
	linux-fsdevel, linux-mtd, viro, linux-kernel

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
<sergei.shtylyov@cogentembedded.com> 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 <viro@zeniv.linux.org.uk>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: David Woodhouse <dwmw2@infradead.org>
> > cc: Richard Weinberger <richard@nod.at>
> > cc: linux-mtd@lists.infradead.org
> [...]
>
> MBR, Sergei
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] jffs2: Fix mounting under new mount API
  2019-11-13 20:38   ` Han Xu
@ 2019-11-14 12:01     ` Hou Tao
  2019-11-28 23:59       ` Joel Stanley
  0 siblings, 1 reply; 6+ messages in thread
From: Hou Tao @ 2019-11-14 12:01 UTC (permalink / raw)
  To: Han Xu, Sergei Shtylyov, Richard Weinberger
  Cc: David Howells, David Woodhouse, linux-fsdevel, linux-mtd, viro,
	linux-kernel

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 <baijiaju1990@gmail.com>
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
> <sergei.shtylyov@cogentembedded.com> 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 <viro@zeniv.linux.org.uk>
>>> Signed-off-by: David Howells <dhowells@redhat.com>
>>> cc: David Woodhouse <dwmw2@infradead.org>
>>> cc: Richard Weinberger <richard@nod.at>
>>> cc: linux-mtd@lists.infradead.org
>> [...]
>>
>> MBR, Sergei
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] jffs2: Fix mounting under new mount API
  2019-11-14 12:01     ` Hou Tao
@ 2019-11-28 23:59       ` Joel Stanley
  0 siblings, 0 replies; 6+ messages in thread
From: Joel Stanley @ 2019-11-28 23:59 UTC (permalink / raw)
  To: Hou Tao
  Cc: Han Xu, Sergei Shtylyov, Richard Weinberger, David Howells,
	David Woodhouse, linux-fsdevel, linux-mtd, viro,
	Linux Kernel Mailing List, Andrew Jeffery

On Thu, 14 Nov 2019 at 12:05, Hou Tao <houtao1@huawei.com> 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 <baijiaju1990@gmail.com>
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-28 23:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 14:21 [PATCH] jffs2: Fix mounting under new mount API David Howells
2019-09-26 14:26 ` Al Viro
2019-09-27  8:38 ` Sergei Shtylyov
2019-11-13 20:38   ` Han Xu
2019-11-14 12:01     ` Hou Tao
2019-11-28 23:59       ` Joel Stanley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).