overlay: fix incorrect diff folder permission with userns and fuse#735
overlay: fix incorrect diff folder permission with userns and fuse#735chaserhkj wants to merge 1 commit intocontainers:mainfrom
Conversation
|
Note: #734 seems to be on a similar but unrelated issue, since it impacts podman but this does not. |
Overlay storage driver creates diff (upper) folder with mapped root IDs when userns is enabled. This is consistent with the behavior of the driver when using native overlayfs, where all lower layer FS will get ID mapped into the userns but upper layer is mounted as is. However when userns is used with fuse-overlayfs, the fuse driver performs id map over the entire FS. Creating diff dir with mapped IDs will cause the fuse layer to map it as nobody in the userns, causing the root folder unwritable in container Fixes cri-o/cri-o#9865 Signed-off-by: Chaser Huang <huangkangjing@gmail.com>
chaserhkj
force-pushed
the
fuse-overlayfs-ownership-fix
branch
from
a24bb5f to
88dc04c
Compare
|
Cc: @giuseppe (I didn’t study this in detail myself yet) |
|
do you have a test/reproducer? Otherwise it is not clear what you are trying to fix |
|
given the discussion in cri-o/cri-o#9865, should we try if the chown is enough for the error you've encountered? |
|
The chown on the engine layer is good to have as it provides a guarantee that the rootfs permissions will be correct and completely independent of any behavior of the storage driver layer may exhibit. I can submit a PR on cri-o side for that change. But I think whether this PR will be obsoleted by the change on the cri-o side depends on what the scope of the storage driver is. The crucial question is: "Is storage driver responsible to ensure the ownership consistencies of the mounted filesystem ?" Since the existing code includes clear code paths and comments that try to ensure consistency (ref: 1 and 2), I assumed the answer to this question is yes. If that is the case then this PR is still warranted since the existing code behavior is not consistent in ownership with fuse-overlayfs. For the reproducer please refer to the cri-o issue linked, and I can add a unit test for this change as well. |
|
can we just move the chown to container-libs and remove it from Podman? In this way it will work for CRI-O as well |
3 participants
Overlay storage driver creates diff (upper) folder with mapped root IDs when userns is enabled. This is consistent with the behavior of the driver when using native overlayfs, where all lower layer FS will get ID mapped into the userns but upper layer is mounted as is.
However when userns is used with fuse-overlayfs, the fuse driver performs id map over the entire FS. Creating diff dir with mapped IDs will cause the fuse layer to map it as nobody in the userns, causing the root folder unwritable in container
Fixes cri-o/cri-o#9865
Variable names have been renamed too to better reflect their intended uses.
Note: This bug only manifests with CRI-O. In podman/libpod an additional pass of chown is performed after mounting is returned from the storage layer (ref). But imo this should be considered a bug from storage layer since fuse-overlayfs logic is handled within this layer as well.