Merge lp:~rlaager/ecryptfs/fix-lp-1574174 into lp:ecryptfs

Proposed by Richard Laager on 2016-04-25
Status: Superseded
Proposed branch: lp:~rlaager/ecryptfs/fix-lp-1574174
Merge into: lp:ecryptfs
Diff against target: 14 lines (+2/-2)
1 file modified
src/utils/ecryptfs-setup-private (+2/-2)
To merge this branch: bzr merge lp:~rlaager/ecryptfs/fix-lp-1574174
Reviewer Review Type Date Requested Status
Tyler Hicks 2016-04-25 Needs Fixing on 2016-04-25
Review via email: mp+292844@code.launchpad.net

This proposal has been superseded by a proposal from 2016-04-25.

Description of the change

Fix improper "already mounted" errors with ZFS

The obvious approach for using ZFS and ecryptfs together involves
creating a dataset like this:
zfs create -o mountpoint=/home/.ecryptfs/USER rpool/home/USER

As a result, /proc/mounts looks like this:
rpool/home/USER /home/.ecryptfs/USER zfs rw,xattr 0 0

ecryptfs-setup-private checked for existing mount points using a grep
which was effectively left-anchored. Unfortunately, this can match the
device column. A space at the beginning of the pattern corrects this.

I applied the same fix to the CRYPTDIR for consistency and correctness,
though I do not believe it was a practical problem in the same way.

To post a comment you must log in.
Tyler Hicks (tyhicks) wrote :

Hi Richard - Thanks for the merge proposal. I could be wrong because I didn't test this but I think adding a leading space to CRYPTDIR would break the "already mounted" check since CRYPTDIR is the device (or source) and will be listed at the beginning of a line in /proc/mounts. The check will never fail even if the CRYPTDIR is already mounted.

review: Needs Fixing
Tyler Hicks (tyhicks) wrote :

I think the check should be:

grep -qs "^$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"

Richard Laager (rlaager) wrote :

Right, I'm dumb. I added that last minute for "consistency". CRYPTDIR isn't the problem for ZFS anyway, MOUNTDIR is. Just drop the CRYPTDIR change from me.

Unmerged revisions

881. By Richard Laager on 2016-04-25

Fix improper "already mounted" errors with ZFS

The obvious approach for using ZFS and ecryptfs together involves
creating a dataset like this:
zfs create -o mountpoint=/home/.ecryptfs/USER rpool/home/USER

As a result, /proc/mounts looks like this:
rpool/home/USER /home/.ecryptfs/USER zfs rw,xattr 0 0

ecryptfs-setup-private checked for existing mount points using a grep
which was effectively left-anchored. Unfortunately, this can match the
device column. A space at the beginning of the pattern corrects this.

I applied the same fix to the CRYPTDIR for consistency and correctness,
though I do not believe it was a practical problem in the same way.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/utils/ecryptfs-setup-private'
2--- src/utils/ecryptfs-setup-private 2016-02-26 23:57:37 +0000
3+++ src/utils/ecryptfs-setup-private 2016-04-25 18:00:16 +0000
4@@ -231,8 +231,8 @@
5 fi
6
7 # Check for active mounts
8-grep -qs "$MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
9-grep -qs "$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
10+grep -qs " $MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
11+grep -qs " $CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
12
13 # Check that the mount point and encrypted directory are empty (skip symlinks).
14 # Perhaps one day we could provide a migration mode (using rsync or something),

Subscribers

People subscribed via source and target branches