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: zfsunlock hook breaks /usr/bin #11162

Merged
merged 1 commit into from
Nov 10, 2020
Merged

initramfs: zfsunlock hook breaks /usr/bin #11162

merged 1 commit into from
Nov 10, 2020

Conversation

pzakha
Copy link
Contributor

@pzakha pzakha commented Nov 5, 2020

When running update-initramfs -u, we get the following output:

$ sudo update-initramfs -u
update-initramfs: Generating /boot/initrd.img-5.4.0-1025-aws
mkdir: cannot create directory ‘/var/tmp/mkinitramfs_x4d2p6//usr/bin’: File exists
cp: failed to access '/var/tmp/mkinitramfs_x4d2p6//usr/bin/dirname': Not a directory
mkdir: cannot create directory ‘/var/tmp/mkinitramfs_x4d2p6//usr/bin’: File exists
cp: failed to access '/var/tmp/mkinitramfs_x4d2p6//usr/bin/env': Not a directory
mkdir: cannot create directory ‘/var/tmp/mkinitramfs_x4d2p6/usr/bin’: File exists
touch: cannot touch '/var/tmp/mkinitramfs_x4d2p6/usr/bin/net': Not a directory
chmod: cannot access '/var/tmp/mkinitramfs_x4d2p6/usr/bin/net': Not a directory

When the initramfs archive is created or updated, it runs a bunch of hooks in /usr/share/initramfs-tools/hooks. A new hook introduced by #10027, zfsunlock, copies a new file in the /usr/bin directory of the initramfs archive. The problem is that when that hook is executed the /usr/bin directory doesn't exist, so instead of copying the script into the /usr/bin directory, it creates a new /usr/bin file with the contents of that script.

Description

The copy_exec() function expects that the full path of the target file is passed rather than just the directory, and will take care of creating the underlying directories if they don't exist.

How Has This Been Tested?

The issue was detected on Ubuntu 18.04 running with zfs on root, using initramfs-tools version 0.130ubuntu3.11. I've modified the hook installed under /usr/share/initramfs-tools/hooks/zfsunlock, re-ran update-initramfs -u and made sure the system can still boot.

Additional thoughts

We may want to consider running all zfs hooks with -e, so that they fail if any sub-command fails. In this case, the errors come from the "zfs" hook, which seems to run after the "zfsunlock" hook.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@pzakha pzakha requested review from grwilson and ahrens November 5, 2020 23:55
@pzakha
Copy link
Contributor Author

pzakha commented Nov 5, 2020

cc: @terem42

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. This is the same as the preferred fix proposed in #10448.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 6, 2020
@terem42
Copy link

terem42 commented Nov 6, 2020

Looks good. This is the same as the preferred fix proposed in #10448.

Agree, there was a missing slash on copy_exec command, covered by earlier fix.
Should be like this:
copy_exec /usr/share/initramfs-tools/zfsunlock /usr/bin/

@pzakha
Copy link
Contributor Author

pzakha commented Nov 6, 2020

@terem42 Just a note regarding copy_exec, the 2nd argument should include the name of the executable rather than just the directory. See the function in initramfs-tools:

# $1 = executable/shared library to copy to initramfs, with dependencies
# $2 (optional) Name for the file on the initramfs
# Location of the image dir is assumed to be $DESTDIR
# We never overwrite the target if it exists.
copy_exec() {

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 9, 2020
@behlendorf behlendorf merged commit 0f66201 into openzfs:master Nov 10, 2020
behlendorf pushed a commit that referenced this pull request Nov 11, 2020
The copy_exec() function expects that the full path of the target
file is passed rather than just the directory, and will take care
of creating the underlying directories if they don't exist.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Pavel Zakharov <[email protected]>
Closes #11162
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
The copy_exec() function expects that the full path of the target 
file is passed rather than just the directory, and will take care 
of creating the underlying directories if they don't exist.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Pavel Zakharov <[email protected]>
Closes openzfs#11162
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
The copy_exec() function expects that the full path of the target 
file is passed rather than just the directory, and will take care 
of creating the underlying directories if they don't exist.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Pavel Zakharov <[email protected]>
Closes openzfs#11162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants