Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initramfs on Debian broken with respect to kernel cmdline option root=zfs:auto #11278

Closed
nachtgeist opened this issue Dec 3, 2020 · 3 comments
Labels
Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@nachtgeist
Copy link
Contributor

nachtgeist commented Dec 3, 2020

System information

Type Version/Name
Distribution Name Debian
Distribution Version buster + buster-backports
Linux Kernel 4.19.0-13-amd64
Architecture amd64
ZFS Version 0.8.5-2~bpo10+1
SPL Version 0.8.5-2~bpo10+1: all

Describe the problem you're observing

  • ZFS as rootfs, setup according to docs
  • supplying grub2 with root=zfs:mypool/myrootdataset works

Describe how to reproduce the problem

  • supplying grub2 with root=zfs:AUTO instead does NOT work, the zfs boot script drops to an initramfs shell

Include any warning/errors/backtraces from the system logs

  1. when suppling grub2 with root=zfs:mypool/myrootdataset zfsdebug=1 I see in the GOOD case:
Begin: Importing pool 'mypool' using defaults ... + ZFS_CMD='/sbin/zpool import -N  '                                                                                                                                 
+ /sbin/zpool import -N mypool                                                                                                                                                                                        
+ ZFS_STDERR=
+ ZFS_ERROR=0
  1. when suppling grub2 with zfsdebug=1 I see in the BAD case:
    (Please note the extra single ticks around '/sbin/zpool import -N ' in this case which do NOT occur in 1.)
Begin: Importing pool 'mypool' using defaults ... + ZFS_CMD='/sbin/zpool import -N  '
+ '/sbin/zpool import -N  ' mypool                                                   # <--- extra ' '
+ ZFS_STDERR='/init: line 237: /sbin/zpool import -N  : not found'
+ ZFS_ERROR=127
+ '[' 127 '!=' 0 ]
+ '[' n '!=' y ]
+ zfs_log_failure_msg 127
+ log_failure_msg 127
+ _log_msg 'Failure: %s\n' 127
+ '[' n '=' y ]
+ printf 'Failure: %s\n' 127
Failure: 127

In-depth analysis and proposed fix

  • The problematic part of the script code used on buster-backports is identical to the current master.
  • I'm using line numbers from the current master do describe code flow.
  • The good case enters mountroot(), branching off from 814 to 841. Nothing special happening here, the calls to import_pool and find_rootfs from 857/858 work fine.
  • The base case enters mountroot(), branching off from 814 to 816. In 830, IFS gets manipulated for a for-loop. The functions import_pool and find_rootfs get called from within that for-loop, with the manipulated IFS in place.
  • Consequently, when the shell is to execute commands from expanded variables, the tokenization no longer happens at word boundaries due to IFS containing whitespace. Now, since IFS is limited to ";" the entire ZFS_CMD is passed as argv[0] to execve() internally and of course there is no such binary as "zpool import -N" located in /sbin.

As a fix, I suggest to prefix the calls to import_pool… and find_rootfs… in 835/836 for the to read:

			IFS="$OLD_IFS" import_pool "$pool"
			IFS="$OLD_IFS" find_rootfs "$pool"

That way, the functions show the expected behaviour and IFS is not manipulated while the for-loop is active and thus not affecting the iteration of the for-loop itself.

@nachtgeist nachtgeist added Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang) labels Dec 3, 2020
@icedream
Copy link

icedream commented Jan 19, 2021

I ran some Debian updates yesterday on a production server with ZFS from buster-backports (ZFS package version 0.8.6-1~bpo10+1 on kernel package version 4.19.0-13-amd64 now) installed and it seems I have now run into this issue as well with the fallback to initramfs shell. Currently working around this by manually selecting an older initramfs/kernel to boot with.

nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 3, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#11278
@nabijaczleweli
Copy link
Contributor

@nachtgeist thanks for this! I couldn't get my head around this when I was fixing both initrd subsystems initially.

Can you try this on your configuration(s) with the changes from #11838? Worked for me (and should in general), but you never know.

@nachtgeist
Copy link
Contributor Author

Worked fine here as well. Thanks!

behlendorf pushed a commit that referenced this issue Apr 7, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #11278
Closes #11838
mcmilk pushed a commit to mcmilk/zfs that referenced this issue Apr 9, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#11278
Closes openzfs#11838
mcmilk pushed a commit to mcmilk/zfs that referenced this issue Apr 9, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#11278
Closes openzfs#11838
adamdmoss pushed a commit to adamdmoss/zfs that referenced this issue Apr 10, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#11278
Closes openzfs#11838
sempervictus pushed a commit to sempervictus/zfs that referenced this issue May 31, 2021
IFS= would break loops in import_pool(), which would fault
any automatic import

Additionally $ZFS_BOOTFS from cmdline would interfere with find_rootfs()

If many pools were present, same thing could happen across multiple
find_rootfs() runs, so bail out early and clean up in error path

Suggested-by: @nachtgeist
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes openzfs#11278
Closes openzfs#11838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

No branches or pull requests

3 participants