[OpenWrt-Devel] Factory startup issues since mount_root / libfstools improvements in Chaos Calmer

[Smith, Pieter]

Hi Rafał and John,

I need your help in confirming a bug that I suspect can be traced back to a
patch you authored / merged. This is what I am observing:

Commit ba01996534d15dc725a2dcc56a59fbfb24b58787 in
git://git.openwrt.org/project/fstools.git seems to have broken startup for
factory-flashed jffs2 OpenWRT systems, causing substantial slowdown in factory
environments.

When a factory-flashed jffs2 on ubi system starts up, the "rootfs_data" volume
contains a deadcode marker. When this happens, mount_root temporarily mounts a
tmpfs overlay, postponing the remounting of the jffs2 overlay until the done
phase of the startup.

The refactoring in ba019965 eliminated an unneeded call to volume_find() when
done() calls jffs2_switch(). Unfortunately the refactoring did not take into
account that volume_identify() has side-effects with the mtd driver
implementation and a second call to volume_identify() on the same struct volume
may render a different result.

This is exactly what happens when the struct volume is passed to jffs2_switch()
by done(). In done(), the volume was identified as FS_DEADCODE by the first
call to volume_identify(). Passing the struct volume to jffs2_switch() then
results in a second call to volume_identify() on the same struct volume. The
second time around the volume is identified as FS_JFFS2 within jffs2_switch(),
resulting in the wrong case being run in the switch. The entire rootfs ends up
yanked out from under the OpenWRT userspace, resulting in everything failing in
unexpected ways. Rebooting recovers userspace, but is quite expensive in
factory environments.

Before the Chaos Calmer release, jffs2_switch() was not stumbling over the
volume_identify() side-effect, because it was retrieving a fresh struct volume
with volume_find() before calling volume_identify().

Adjusting the jffs2_switch() prototype so that done() can forward the result
of volume_identify() to jffs2_switch() functionally resolves the issue, but I
am not sure if this is the best approach (See attached patch below).

Will you please assist?

Regards,
Pieter Smith
Sr. Software Designer


From d0dcde51ec68497882b4d0138a019c049e699177 Mon Sep 17 00:00:00 2001
From: Pieter Smith <pieter.smith at philips.com>
Date: Mon, 27 Mar 2017 17:18:55 +0200
Subject: [PATCH] fix startup with empty "rootfs_data" UBI volume

Commit ba019965 broke startup for a factory-flashed jffs2-on-ubi systems,
causing substantial slowdown in factory environments.

When a factory-flashed jffs2 on ubi system starts up, the "rootfs_data" volume
contains a deadcode marker. When this happens, mount_root temporarily mounts a
tmpfs overlay, postponing the remounting of the jffs2 overlay until the done
phase of the startup.

The refactoring in ba019965 eliminated an "unneeded" call to volume_find() when
done() called jffs2_switch(). Unfortunately the refactoring did not take into
account that volume_identify() has side-effects with the mtd driver
implementation and a second call to volume_identify() on the same struct volume
may render a different result.

This is exactly what happens when the struct volume is passed to jffs2_switch()
by done(). In done(), the volume was identified as FS_DEADCODE by the first
call to volume_identify(). Passing the struct volume to jffs2_switch() then
results in a second call to volume_identify() on the same struct volume. The
second time around the volume is identified as FS_JFFS2 within jffs2_switch(),
resulting in the wrong case being run in the switch. The entire rootfs ends up
yanked out from under the OpenWRT userspace, resulting in everything failing in
unexpected ways.

This patch solves the issue by adjusting jffs2_switch() to also receive the
result of the first volume_identify() call.
---
 libfstools/libfstools.h | 2 +-
 libfstools/overlay.c    | 4 ++--
 mount_root.c            | 5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/libfstools/libfstools.h b/libfstools/libfstools.h
index 940c504..36ba5fb 100644
--- a/libfstools/libfstools.h
+++ b/libfstools/libfstools.h
@@ -51,7 +51,7 @@ extern char* find_mount(char *mp);
 extern char* find_mount_point(char *block, int mtd_only);
 extern int find_filesystem(char *fs);

-extern int jffs2_switch(struct volume *v);
+extern int jffs2_switch(struct volume *v, int ident);

 extern int handle_whiteout(const char *dir);
 extern void foreachdir(const char *dir, int (*cb)(const char*));
diff --git a/libfstools/overlay.c b/libfstools/overlay.c
index 7a62c23..3972af1 100644
--- a/libfstools/overlay.c
+++ b/libfstools/overlay.c
@@ -193,7 +193,7 @@ handle_whiteout(const char *dir)
 }

 int
-jffs2_switch(struct volume *v)
+jffs2_switch(struct volume *v, int ident)
 {
 char *mp;
 char buf[32];
@@ -218,7 +218,7 @@ jffs2_switch(struct volume *v)
 system(buf);
 }

-switch (volume_identify(v)) {
+switch (ident) {
 case FS_NONE:
 ULOG_ERR("no jffs2 marker found\n");
 /* fall through */
diff --git a/mount_root.c b/mount_root.c
index 1335f2b..81089a2 100644
--- a/mount_root.c
+++ b/mount_root.c
@@ -100,10 +100,11 @@ done(int argc, char *argv[1])
 if (!v)
 return -1;

-switch (volume_identify(v)) {
+int ident = volume_identify(v);
+switch (ident) {
 case FS_NONE:
 case FS_DEADCODE:
-return jffs2_switch(v);
+return jffs2_switch(v, ident);

 case FS_JFFS2:
 case FS_UBIFS:
--
2.7.4


________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel