Initramfs FSID altered in 3.14

PATCH Superseded Dave Reisner 8 participants 13 messages Apr 3, 2014

Subsystems (2)

Stable branch VFS

lore

Share & tools

lore

Dave Reisner

author

2014-04-03 17:57 UTC #0 lore raw

Hi,

[This is a repost of a G+ post at Tejun's request]

With Linux 3.14, you might notice in /proc/self/mountinfo that your root's parent FSID is now 0, instead of the 1 that it's been for the last N years. Tejun wrote the change (9e30cc9595303b27b48) that caused this, but the change comes in a rather innocuous way. Instead of an internal kernel mount of sysfs being assigned 0, it's now the initramfs.

So far, this has already caused switch_root and findmnt (from util-linux) to break, cp (from coreutils) to break when using the -x flag in early userspace, and it's also been pointed out that systemd's readahead code makes assumptions about a device number of 0.

Are we now supposed to go and change all the assumptions in userspace about 0 being special? I'm conflicted. The kernel isn't supposed to break userspace, but it seems to me that FSIDs were never something to rely on -- similar to the block device numbering scheme.

Cheers,
Dave

Thomas Bächler

2014-04-03 18:14 UTC #1 lore raw

Am 03.04.2014 19:57, schrieb Dave Reisner:

Quoted 15 lines

Hi,
[This is a repost of a G+ post at Tejun's request]
With Linux 3.14, you might notice in /proc/self/mountinfo that your root's parent FSID is now 0, instead of the 1 that it's been for the last N years. Tejun wrote the change (9e30cc9595303b27b48) that caused this, but the change comes in a rather innocuous way. Instead of an internal kernel mount of sysfs being assigned 0, it's now the initramfs.
So far, this has already caused switch_root and findmnt (from util-linux) to break, cp (from coreutils) to break when using the -x flag in early userspace, and it's also been pointed out that systemd's readahead code makes assumptions about a device number of 0.

Quoted 4 lines

Are we now supposed to go and change all the assumptions in userspace about 0 being special? I'm conflicted. The kernel isn't supposed to break userspace, but it seems to me that FSIDs were never something to rely on -- similar to the block device numbering scheme.

Most of these bugs were not caused by rootfs' FSID being different from 1, but rather because there was a file system with FSID 0.

Only util-linux/switch_root assumed that rootfs always had exactly FSID 1 - which is IMO a wrong assumption.

However, tt seems that people have been assuming that st_dev > 0 for a while. If we want to revert this in the kernel, this patch (untested) should be sufficient:

fs/super.c +1 -1

index 80d5cf2..d9fddde 100644
@@ -802,7 +802,7 @@ void emergency_remount(void)
 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+static int unnamed_dev_start = 1; /* don't bother trying below it */
 int get_anon_bdev(dev_t *p)
 {

Tejun Heo

2014-04-03 19:16 UTC #2 lore raw

Hello,

Quoted 15 lines · Thomas Bächler

Most of these bugs were not caused by rootfs' FSID being different from 1, but rather because there was a file system with FSID 0.
Only util-linux/switch_root assumed that rootfs always had exactly FSID 1 - which is IMO a wrong assumption.
However, tt seems that people have been assuming that st_dev > 0 for a while. If we want to revert this in the kernel, this patch (untested) should be sufficient:
fs/super.c +0 -0
index 80d5cf2..d9fddde 100644

fs/super.c void emergency_remount(void) · line 806

Quoted 6 lines

fs/super.c +1 -1

@@ -802,7 +802,7 @@ void emergency_remount(void)
 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+static int unnamed_dev_start = 1; /* don't bother trying below it */
 int get_anon_bdev(dev_t *p)
 {

Alexandre, this is the one line change that should fix it. Can you please test it?

Thomas, can you please write proper patch description with reference to the following thread and stable # 3.14 tag?

http://lkml.kernel.org/g/CAPEhTTFP3N-ReasmgL5n82mve8p8M3crqmaMvzV+F2p5JCSRbQ@mail.gmail.com

Thanks.

Signature · 1 line

tejun

Theodore Ts'o

2014-04-03 19:20 UTC #3 lore raw

I agree with Tejun that we should fix it up, and not force distributions to have to send out emergency releases of various userspace utilities --- especially since the fix is so simple.

- Ted

Alexandre Demers

2014-04-03 22:02 UTC #4 lore raw

I'll be testing it later tonight. It was either that or I would have been completing my Qemu image. ;)

Stay tuned.

Alexandre

Quoted 5 lines · Theodore Ts'o

I agree with Tejun that we should fix it up, and not force distributions to have to send out emergency releases of various userspace utilities --- especially since the fix is so simple.
- Ted

Thomas Bächler

2014-04-03 19:49 UTC #5 lore raw

PATCH 1/1 fs: Don't return 0 from get_anon_bdev

In reply to Tejun Heo

Commit 9e30cc9595303b27b48 removed an internal mount. This has the side-effect that rootfs now has FSID 0. Many userspace utilities assume that st_dev in struct stat is never 0, so this change breaks a number of tools in early userspace.

Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.

References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557

Cc 3.14 <[email protected]>

Signed-off-by Thomas Bächler <[email protected]>

---
 fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/super.c +1 -1

index 80d5cf2..d9fddde 100644
@@ -802,7 +802,7 @@ void emergency_remount(void)
 
 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+static int unnamed_dev_start = 1; /* don't bother trying below it */
 
 int get_anon_bdev(dev_t *p)
 {

Signature · 1 line

1.9.1

Tejun Heo

Acked-by on v1 1/1

2014-04-03 19:51 UTC #6 lore raw

Re: [PATCH] fs: Don't return 0 from get_anon_bdev

Quoted 6 lines · Thomas Bï¿½chler

Commit 9e30cc9595303b27b48 removed an internal mount. This has the side-effect that rootfs now has FSID 0. Many userspace utilities assume that st_dev in struct stat is never 0, so this change breaks a number of tools in early userspace.

Quoted 7 lines

Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.
References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557
Cc 3.14 <[email protected]>
Signed-off-by Thomas Bï¿½chler <[email protected]>

Provided Alexandre confirms the fix.

Acked-by Tejun Heo <[email protected]>

Quoted 9 lines

---
 fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/super.c +0 -0

index 80d5cf2..d9fddde 100644

fs/super.c void emergency_remount(void) · line 805

Quoted 5 lines

fs/super.c +1 -1

@@ -802,7 +802,7 @@ void emergency_remount(void)
 
 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+static int unnamed_dev_start = 1; /* don't bother trying below it */

Maybe a comment saying that userland considers fsid 0 to be invalid would be nice?

Thanks.

Signature · 1 line

tejun

Thomas Bächler

2014-04-03 19:55 UTC #7 lore raw

PATCH v2 1/1 fs: Don't return 0 from get_anon_bdev

Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.

References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557

Cc 3.14 <[email protected]>

Signed-off-by Thomas Bächler <[email protected]>

---
 fs/super.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

fs/super.c +4 -1

index 80d5cf2..7624267 100644
@@ -802,7 +802,10 @@ void emergency_remount(void)
 
 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+/* Many userspace utilities consider an FSID of 0 invalid.
+ * Always return at least 1 from get_anon_bdev.
+ */
+static int unnamed_dev_start = 1;
 
 int get_anon_bdev(dev_t *p)
 {

Signature · 1 line

1.9.1

Greg KH

maintainer

2014-04-03 20:30 UTC #8 lore raw

Re: [PATCHv2] fs: Don't return 0 from get_anon_bdev

Quoted 23 lines · Thomas Bächler

Commit 9e30cc9595303b27b48 removed an internal mount. This has the side-effect that rootfs now has FSID 0. Many userspace utilities assume that st_dev in struct stat is never 0, so this change breaks a number of tools in early userspace.
Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.
References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557
Cc 3.14 <[email protected]>
Signed-off-by Thomas Bächler <[email protected]>
---
 fs/super.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
fs/super.c +0 -0
index 80d5cf2..7624267 100644
@@ -802,7 +802,10 @@ void emergency_remount(void)
 

fs/super.c void emergency_remount(void) · line 811

Quoted 10 lines

fs/super.c +4 -1

 static DEFINE_IDA(unnamed_dev_ida);
 static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+/* Many userspace utilities consider an FSID of 0 invalid.
+ * Always return at least 1 from get_anon_bdev.
+ */
+static int unnamed_dev_start = 1;
 
 int get_anon_bdev(dev_t *p)
 {

Very nice, I'll go queue this one up instead of the other patch from Tejun.

greg k-h

H. Peter Anvin

Acked-by on v1 1/1

2014-04-03 20:21 UTC #9 lore raw

Re: [PATCH] fs: Don't return 0 from get_anon_bdev

In reply to Thomas Bächler

Quoted 9 lines · Thomas Bächler

Commit 9e30cc9595303b27b48 removed an internal mount. This has the side-effect that rootfs now has FSID 0. Many userspace utilities assume that st_dev in struct stat is never 0, so this change breaks a number of tools in early userspace.
Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.

Quoted 8 lines

References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557
Cc 3.14 <[email protected]>
Signed-off-by Thomas Bächler <[email protected]>
---
 fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by H. Peter Anvin <[email protected]>

It is worth noting that zero has been documented as a null device number since at least 1995, probably more like 1993.

-hpa

Alexandre Demers

Tested-by on v1 1/1

2014-04-03 23:41 UTC #10 lore raw

Re: [PATCH] fs: Don't return 0 from get_anon_bdev

In reply to Thomas Bächler

It works over here, tested on 3.14-rc8 which was previously failing. You have my

Tested-by Alexandre Demers

Alexandre Demers

Quoted 30 lines · Thomas Bächler

Commit 9e30cc9595303b27b48 removed an internal mount. This has the side-effect that rootfs now has FSID 0. Many userspace utilities assume that st_dev in struct stat is never 0, so this change breaks a number of tools in early userspace.
Since we don't know how many userspace programs are affected, make sure that FSID is at least 1.
References: http://article.gmane.org/gmane.linux.kernel/1666905
References: http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/8557
Cc 3.14 <[email protected]>
Signed-off-by Thomas Bächler <[email protected]>
---
  fs/super.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
fs/super.c +1 -1
index 80d5cf2..d9fddde 100644
@@ -802,7 +802,7 @@ void emergency_remount(void)
 
  static DEFINE_IDA(unnamed_dev_ida);
  static DEFINE_SPINLOCK(unnamed_dev_lock);/* protects the above */
-static int unnamed_dev_start = 0; /* don't bother trying below it */
+static int unnamed_dev_start = 1; /* don't bother trying below it */
 
  int get_anon_bdev(dev_t *p)
  {

Pádraig Brady

2014-04-03 18:57 UTC #11 lore raw

In reply to Dave Reisner

Quoted 10 lines · Dave Reisner

Hi,
[This is a repost of a G+ post at Tejun's request]
With Linux 3.14, you might notice in /proc/self/mountinfo that your root's parent FSID is now 0, instead of the 1 that it's been for the last N years. Tejun wrote the change (9e30cc9595303b27b48) that caused this, but the change comes in a rather innocuous way. Instead of an internal kernel mount of sysfs being assigned 0, it's now the initramfs.

Quoted 4 lines

So far, this has already caused switch_root and findmnt (from util-linux) to break, cp (from coreutils) to break when using the -x flag in early userspace, and it's also been pointed out that systemd's readahead code makes assumptions about a device number of 0.

For reference we've changed coreutils not to assume 0 is an invalid device ID:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=d0294ff3

Quoted 4 lines

Are we now supposed to go and change all the assumptions in userspace about 0 being special? I'm conflicted. The kernel isn't supposed to break userspace, but it seems to me that FSIDs were never something to rely on -- similar to the block device numbering scheme.

I would say the kernel doesn't care what the value is, so to ease compat worries just use >= 1.

cheers,
Pádraig

Tejun Heo

2014-04-03 19:13 UTC #12 lore raw

In reply to Dave Reisner

Hello, Dave.

Quoted 11 lines · Dave Reisner

With Linux 3.14, you might notice in /proc/self/mountinfo that your root's parent FSID is now 0, instead of the 1 that it's been for the last N years. Tejun wrote the change (9e30cc9595303b27b48) that caused this, but the change comes in a rather innocuous way. Instead of an internal kernel mount of sysfs being assigned 0, it's now the initramfs.
So far, this has already caused switch_root and findmnt (from util-linux) to break, cp (from coreutils) to break when using the -x flag in early userspace, and it's also been pointed out that systemd's readahead code makes assumptions about a device number of 0.

Quoted 4 lines

Are we now supposed to go and change all the assumptions in userspace about 0 being special? I'm conflicted. The kernel isn't supposed to break userspace, but it seems to me that FSIDs were never something to rely on -- similar to the block device numbering scheme.

I think this is the same problem Alexandre Demers reported. Arch was failing to boot with the commit. There's already a patch pending to reinstate the internal mount but I think what Thomas is proposing - just starting allocating FSID from 1 is a better solution. Alexandre, can you please test Thomas' patch?

Thanks.

Signature · 1 line

tejun