diff mbox series

[OpenWrt-Devel] jffs2: Fix use of uninitialized delayed_work, lockdep breakage

Message ID 20181019085920.16735-1-daniel.santos@pobox.com
State Changes Requested
Delegated to: John Crispin
Headers show
Series [OpenWrt-Devel] jffs2: Fix use of uninitialized delayed_work, lockdep breakage | expand

Commit Message

Daniel Santos Oct. 19, 2018, 8:59 a.m. UTC
I've sent this one upstream.  This patch is critical if you want to run
with "prove lock correctness" (lockdep) and you happen to have certain
mtd devices.  The misuse of the uninitialized object is undefined
behaviour, but being zeroed it does not appear to have actually broken
anything other than the lockdep engine.

Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
---
 ...se-of-uninitialized-delayed_work-lockdep-.patch | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch

Comments

Hauke Mehrtens Nov. 24, 2018, 2:05 p.m. UTC | #1
On 10/19/18 10:59 AM, Daniel Santos wrote:
> I've sent this one upstream.  This patch is critical if you want to run
> with "prove lock correctness" (lockdep) and you happen to have certain
> mtd devices.  The misuse of the uninitialized object is undefined
> behaviour, but being zeroed it does not appear to have actually broken
> anything other than the lockdep engine.
> 
> Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
> ---
>  ...se-of-uninitialized-delayed_work-lockdep-.patch | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch
> 
> diff --git a/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch b/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch
> new file mode 100644
> index 0000000000..5b2184f340
> --- /dev/null
> +++ b/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch
> @@ -0,0 +1,64 @@
> +From 1d9ab2b7c27a10acfa437c561f276d52d563e058 Mon Sep 17 00:00:00 2001
> +From: Daniel Santos <daniel.santos@pobox.com>
> +Date: Fri, 19 Oct 2018 01:50:20 -0500
> +Subject: jffs2: Fix use of uninitialized delayed_work, lockdep breakage
> +
> +jffs2_sync_fs makes the assumption that if CONFIG_JFFS2_FS_WRITEBUFFER
> +is defined then a write buffer is available and has been initialized.
> +However, this does is not the case when the mtd device has no
> +out-of-band buffer:
> +
> +int jffs2_nand_flash_setup(struct jffs2_sb_info *c)
> +{
> +        if (!c->mtd->oobsize)
> +                return 0;
> +...
> +
> +The resulting call to cancel_delayed_work_sync passing a uninitialized
> +(but zeroed) delayed_work struct forces lockdep to become disabled.
> +
> +[   90.050639] overlayfs: upper fs does not support tmpfile.
> +[   90.652264] INFO: trying to register non-static key.
> +[   90.662171] the code is fine but needs lockdep annotation.
> +[   90.673090] turning off the locking correctness validator.
> +[   90.684021] CPU: 0 PID: 1762 Comm: mount_root Not tainted 4.14.63 #0
> +[   90.696672] Stack : 00000000 00000000 80d8f6a2 00000038 805f0000 80444600 8fe364f4 805dfbe7
> +[   90.713349]         80563a30 000006e2 8068370c 00000001 00000000 00000001 8e2fdc48 ffffffff
> +[   90.730020]         00000000 00000000 80d90000 00000000 00000106 00000000 6465746e 312e3420
> +[   90.746690]         6b636f6c 03bf0000 f8000000 20676e69 00000000 80000000 00000000 8e2c2a90
> +[   90.763362]         80d90000 00000001 00000000 8e2c2a90 00000003 80260dc0 08052098 80680000
> +[   90.780033]         ...
> +[   90.784902] Call Trace:
> +[   90.789793] [<8000f0d8>] show_stack+0xb8/0x148
> +[   90.798659] [<8005a000>] register_lock_class+0x270/0x55c
> +[   90.809247] [<8005cb64>] __lock_acquire+0x13c/0xf7c
> +[   90.818964] [<8005e314>] lock_acquire+0x194/0x1dc
> +[   90.828345] [<8003f27c>] flush_work+0x200/0x24c
> +[   90.837374] [<80041dfc>] __cancel_work_timer+0x158/0x210
> +[   90.847958] [<801a8770>] jffs2_sync_fs+0x20/0x54
> +[   90.857173] [<80125cf4>] iterate_supers+0xf4/0x120
> +[   90.866729] [<80158fc4>] sys_sync+0x44/0x9c
> +[   90.875067] [<80014424>] syscall_common+0x34/0x58
> +
> +Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
> +---
> + fs/jffs2/super.c | 3 ++-
> + 1 file changed, 2 insertions(+), 1 deletion(-)
> +
> +diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> +index 793ad30970ff..cae4ecda3c50 100644
> +--- a/fs/jffs2/super.c
> ++++ b/fs/jffs2/super.c
> +@@ -101,7 +101,8 @@ static int jffs2_sync_fs(struct super_block *sb, int wait)
> + 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
> + 
> + #ifdef CONFIG_JFFS2_FS_WRITEBUFFER
> +-	cancel_delayed_work_sync(&c->wbuf_dwork);
> ++	if (jffs2_is_writebuffered(c))
> ++		cancel_delayed_work_sync(&c->wbuf_dwork);
> + #endif
> + 
> + 	mutex_lock(&c->alloc_sem);
> +-- 
> +2.16.4
> +
> 

We would like to reduce the number of patches we ship in OpenWrt on top
of the mainline Linux kernel.

Please send this to the upstream Maintainer of the jffs driver in the
mainline Linux kernel for integration into the mainline Linux kernel. If
this was accepted in some maintainers tree please provide the link to
the commit.

Hauke
Daniel Santos Dec. 3, 2018, 10:56 p.m. UTC | #2
On 11/24/18 8:05 AM, Hauke Mehrtens wrote:
> On 10/19/18 10:59 AM, Daniel Santos wrote:
> [snip]
> We would like to reduce the number of patches we ship in OpenWrt on top
> of the mainline Linux kernel.
>
> Please send this to the upstream Maintainer of the jffs driver in the
> mainline Linux kernel for integration into the mainline Linux kernel. If
> this was accepted in some maintainers tree please provide the link to
> the commit.
>
> Hauke
>
Hello Hauke,

Here ya go:
http://git.infradead.org/linux-mtd.git/commit/a788c5272769ddbcdbab297cf386413eeac04463

Daniel
John Crispin Dec. 3, 2018, 11:10 p.m. UTC | #3
On 03/12/2018 23:56, Daniel Santos wrote:
> On 11/24/18 8:05 AM, Hauke Mehrtens wrote:
>> On 10/19/18 10:59 AM, Daniel Santos wrote:
>> [snip]
>> We would like to reduce the number of patches we ship in OpenWrt on top
>> of the mainline Linux kernel.
>>
>> Please send this to the upstream Maintainer of the jffs driver in the
>> mainline Linux kernel for integration into the mainline Linux kernel. If
>> this was accepted in some maintainers tree please provide the link to
>> the commit.
>>
>> Hauke
>>
> Hello Hauke,
>
> Here ya go:
> http://git.infradead.org/linux-mtd.git/commit/a788c5272769ddbcdbab297cf386413eeac04463
>
> Daniel
>
>

well done !

> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch b/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch
new file mode 100644
index 0000000000..5b2184f340
--- /dev/null
+++ b/target/linux/generic/pending-4.14/142-jffs2-Fix-use-of-uninitialized-delayed_work-lockdep-.patch
@@ -0,0 +1,64 @@ 
+From 1d9ab2b7c27a10acfa437c561f276d52d563e058 Mon Sep 17 00:00:00 2001
+From: Daniel Santos <daniel.santos@pobox.com>
+Date: Fri, 19 Oct 2018 01:50:20 -0500
+Subject: jffs2: Fix use of uninitialized delayed_work, lockdep breakage
+
+jffs2_sync_fs makes the assumption that if CONFIG_JFFS2_FS_WRITEBUFFER
+is defined then a write buffer is available and has been initialized.
+However, this does is not the case when the mtd device has no
+out-of-band buffer:
+
+int jffs2_nand_flash_setup(struct jffs2_sb_info *c)
+{
+        if (!c->mtd->oobsize)
+                return 0;
+...
+
+The resulting call to cancel_delayed_work_sync passing a uninitialized
+(but zeroed) delayed_work struct forces lockdep to become disabled.
+
+[   90.050639] overlayfs: upper fs does not support tmpfile.
+[   90.652264] INFO: trying to register non-static key.
+[   90.662171] the code is fine but needs lockdep annotation.
+[   90.673090] turning off the locking correctness validator.
+[   90.684021] CPU: 0 PID: 1762 Comm: mount_root Not tainted 4.14.63 #0
+[   90.696672] Stack : 00000000 00000000 80d8f6a2 00000038 805f0000 80444600 8fe364f4 805dfbe7
+[   90.713349]         80563a30 000006e2 8068370c 00000001 00000000 00000001 8e2fdc48 ffffffff
+[   90.730020]         00000000 00000000 80d90000 00000000 00000106 00000000 6465746e 312e3420
+[   90.746690]         6b636f6c 03bf0000 f8000000 20676e69 00000000 80000000 00000000 8e2c2a90
+[   90.763362]         80d90000 00000001 00000000 8e2c2a90 00000003 80260dc0 08052098 80680000
+[   90.780033]         ...
+[   90.784902] Call Trace:
+[   90.789793] [<8000f0d8>] show_stack+0xb8/0x148
+[   90.798659] [<8005a000>] register_lock_class+0x270/0x55c
+[   90.809247] [<8005cb64>] __lock_acquire+0x13c/0xf7c
+[   90.818964] [<8005e314>] lock_acquire+0x194/0x1dc
+[   90.828345] [<8003f27c>] flush_work+0x200/0x24c
+[   90.837374] [<80041dfc>] __cancel_work_timer+0x158/0x210
+[   90.847958] [<801a8770>] jffs2_sync_fs+0x20/0x54
+[   90.857173] [<80125cf4>] iterate_supers+0xf4/0x120
+[   90.866729] [<80158fc4>] sys_sync+0x44/0x9c
+[   90.875067] [<80014424>] syscall_common+0x34/0x58
+
+Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
+---
+ fs/jffs2/super.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
+index 793ad30970ff..cae4ecda3c50 100644
+--- a/fs/jffs2/super.c
++++ b/fs/jffs2/super.c
+@@ -101,7 +101,8 @@ static int jffs2_sync_fs(struct super_block *sb, int wait)
+ 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
+ 
+ #ifdef CONFIG_JFFS2_FS_WRITEBUFFER
+-	cancel_delayed_work_sync(&c->wbuf_dwork);
++	if (jffs2_is_writebuffered(c))
++		cancel_delayed_work_sync(&c->wbuf_dwork);
+ #endif
+ 
+ 	mutex_lock(&c->alloc_sem);
+-- 
+2.16.4
+