31 Oct, 2016

1 commit

  • tmpfs doesn't have ->get_acl() because it only uses cached acls.

    This fixes the acl tests in pjdfstest when tmpfs is used as the upper layer
    of the overlay.

    Reported-by: Amir Goldstein
    Signed-off-by: Miklos Szeredi
    Fixes: 39a25b2b3762 ("ovl: define ->get_acl() for overlay inodes")
    Cc: # v4.8

    Miklos Szeredi
     

15 Oct, 2016

1 commit

  • Pull overlayfs updates from Miklos Szeredi:
    "This update contains fixes to the "use mounter's permission to access
    underlying layers" area, and miscellaneous other fixes and cleanups.

    No new features this time"

    * 'overlayfs-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs:
    ovl: use vfs_get_link()
    vfs: add vfs_get_link() helper
    ovl: use generic_readlink
    ovl: explain error values when removing acl from workdir
    ovl: Fix info leak in ovl_lookup_temp()
    ovl: during copy up, switch to mounter's creds early
    ovl: lookup: do getxattr with mounter's permission
    ovl: copy_up_xattr(): use strnlen

    Linus Torvalds
     

14 Oct, 2016

2 commits


11 Oct, 2016

1 commit

  • Pull vfs xattr updates from Al Viro:
    "xattr stuff from Andreas

    This completes the switch to xattr_handler ->get()/->set() from
    ->getxattr/->setxattr/->removexattr"

    * 'work.xattr' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
    vfs: Remove {get,set,remove}xattr inode operations
    xattr: Stop calling {get,set,remove}xattr inode operations
    vfs: Check for the IOP_XATTR flag in listxattr
    xattr: Add __vfs_{get,set,remove}xattr helpers
    libfs: Use IOP_XATTR flag for empty directory handling
    vfs: Use IOP_XATTR flag for bad-inode handling
    vfs: Add IOP_XATTR inode operations flag
    vfs: Move xattr_resolve_name to the front of fs/xattr.c
    ecryptfs: Switch to generic xattr handlers
    sockfs: Get rid of getxattr iop
    sockfs: getxattr: Fail with -EOPNOTSUPP for invalid attribute names
    kernfs: Switch to generic xattr handlers
    hfs: Switch to generic xattr handlers
    jffs2: Remove jffs2_{get,set,remove}xattr macros
    xattr: Remove unnecessary NULL attribute name check

    Linus Torvalds
     

08 Oct, 2016

2 commits


22 Sep, 2016

1 commit

  • inode_change_ok() will be resposible for clearing capabilities and IMA
    extended attributes and as such will need dentry. Give it as an argument
    to inode_change_ok() instead of an inode. Also rename inode_change_ok()
    to setattr_prepare() to better relect that it does also some
    modifications in addition to checks.

    Reviewed-by: Christoph Hellwig
    Signed-off-by: Jan Kara

    Jan Kara
     

19 Sep, 2016

1 commit

  • Now, we have the notion that copy up of a file is done with the creds
    of mounter of overlay filesystem (as opposed to task). Right now before
    we switch creds, we do some vfs_getattr() operations in the context of
    task and that itself can fail. We should do that getattr() using the
    creds of mounter instead.

    So this patch switches to mounter's creds early during copy up process so
    that even vfs_getattr() is done with mounter's creds.

    Do not call revert_creds() unless we have already called
    ovl_override_creds(). [Reported by Arnd Bergmann]

    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     

01 Sep, 2016

6 commits

  • Be defensive about what underlying fs provides us in the returned xattr
    list buffer. If it's not properly null terminated, bail out with a warning
    insead of BUG.

    Signed-off-by: Miklos Szeredi
    Cc:

    Miklos Szeredi
     
  • Now that overlayfs has xattr handlers for iop->{set,remove}xattr, use
    those same handlers for iop->getxattr as well.

    Signed-off-by: Andreas Gruenbacher
    Signed-off-by: Miklos Szeredi

    Andreas Gruenbacher
     
  • Commit d837a49bd57f ("ovl: fix POSIX ACL setting") switches from
    iop->setxattr from ovl_setxattr to generic_setxattr, so switch from
    ovl_removexattr to generic_removexattr as well. As far as permission
    checking goes, the same rules should apply in either case.

    While doing that, rename ovl_setxattr to ovl_xattr_set to indicate that
    this is not an iop->setxattr implementation and remove the unused inode
    argument.

    Move ovl_other_xattr_set above ovl_own_xattr_set so that they match the
    order of handlers in ovl_xattr_handlers.

    Signed-off-by: Andreas Gruenbacher
    Fixes: d837a49bd57f ("ovl: fix POSIX ACL setting")
    Signed-off-by: Miklos Szeredi

    Andreas Gruenbacher
     
  • Make sure ovl_own_xattr_handler only matches attribute names starting
    with "overlay.", not "overlayXXX".

    Signed-off-by: Andreas Gruenbacher
    Fixes: d837a49bd57f ("ovl: fix POSIX ACL setting")
    Signed-off-by: Miklos Szeredi

    Andreas Gruenbacher
     
  • Some operations (setxattr/chmod) can make the cached acl stale. We either
    need to clear overlay's acl cache for the affected inode or prevent acl
    caching on the overlay altogether. Preventing caching has the following
    advantages:

    - no double caching, less memory used

    - overlay cache doesn't go stale when fs clears it's own cache

    Possible disadvantage is performance loss. If that becomes a problem
    get_acl() can be optimized for overlayfs.

    This patch disables caching by pre setting i_*acl to a value that

    - has bit 0 set, so is_uncached_acl() will return true

    - is not equal to ACL_NOT_CACHED, so get_acl() will not overwrite it

    The constant -3 was chosen for this purpose.

    Fixes: 39a25b2b3762 ("ovl: define ->get_acl() for overlay inodes")
    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • Instead of calling ->get_acl() directly, use get_acl() to get the cached
    value.

    We will have the acl cached on the underlying inode anyway, because we do
    permission checking on the both the overlay and the underlying fs.

    So, since we already have double caching, this improves performance without
    any cost.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     

08 Aug, 2016

1 commit

  • When a copy up of a directory occurs which has the opaque xattr set, the
    xattr remains in the upper directory. The immediate behavior with overlayfs
    is that the upper directory is not treated as opaque, however after a
    remount the opaque flag is used and upper directory is treated as opaque.
    This causes files created in the lower layer to be hidden when using
    multiple lower directories.

    Fix by not copying up the opaque flag.

    To reproduce:

    ----8
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=151291
    Signed-off-by: Miklos Szeredi
    Cc:

    Miklos Szeredi
     

29 Jul, 2016

13 commits

  • Right now we remove MAY_WRITE/MAY_APPEND bits from mask if realfile is on
    lower/. This is done as files on lower will never be written and will be
    copied up. But to copy up a file, mounter should have MAY_READ permission
    otherwise copy up will fail. So set MAY_READ in mask when MAY_WRITE is
    reset.

    Dan Walsh noticed this when he did access(lowerfile, W_OK) and it returned
    True (context mounts) but when he tried to actually write to file, it
    failed as mounter did not have permission on lower file.

    [SzM] don't set MAY_READ if only MAY_APPEND is set without MAY_WRITE; this
    won't trigger a copy-up.

    Reported-by: Dan Walsh
    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Right now if file is on lower/, we remove MAY_WRITE/MAY_APPEND bits from
    mask as lower/ will never be written and file will be copied up. But this
    is not true for special files. These files are not copied up and are opened
    in place. So don't dilute the checks for these types of files.

    Reported-by: Dan Walsh
    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Setting POSIX ACL needs special handling:

    1) Some permission checks are done by ->setxattr() which now uses mounter's
    creds ("ovl: do operations on underlying file system in mounter's
    context"). These permission checks need to be done with current cred as
    well.

    2) Setting ACL can fail for various reasons. We do not need to copy up in
    these cases.

    In the mean time switch to using generic_setxattr.

    [Arnd Bergmann] Fix link error without POSIX ACL. posix_acl_from_xattr()
    doesn't have a 'static inline' implementation when CONFIG_FS_POSIX_ACL is
    disabled, and I could not come up with an obvious way to do it.

    This instead avoids the link error by defining two sets of ACL operations
    and letting the compiler drop one of the two at compile time depending
    on CONFIG_FS_POSIX_ACL. This avoids all references to the ACL code,
    also leading to smaller code.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • Inode attributes are copied up to overlay inode (uid, gid, mode, atime,
    mtime, ctime) so generic code using these fields works correcty. If a hard
    link is created in overlayfs separate inodes are allocated for each link.
    If chmod/chown/etc. is performed on one of the links then the inode
    belonging to the other ones won't be updated.

    This patch attempts to fix this by sharing inodes for hard links.

    Use inode hash (with real inode pointer as a key) to make sure overlay
    inodes are shared for hard links on upper. Hard links on lower are still
    split (which is not user observable until the copy-up happens, see
    Documentation/filesystems/overlayfs.txt under "Non-standard behavior").

    The inode is only inserted in the hash if it is non-directoy and upper.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • To get from overlay inode to real inode we currently use 'struct
    ovl_entry', which has lifetime connected to overlay dentry. This is okay,
    since each overlay dentry had a new overlay inode allocated.

    Following patch will break that assumption, so need to leave out ovl_entry.
    This patch stores the real inode directly in i_private, with the lowest bit
    used to indicate whether the inode is upper or lower.

    Lifetime rules remain, using ovl_inode_real() must only be done while
    caller holds ref on overlay dentry (and hence on real dentry), or within
    RCU protected regions.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • The error is due to RCU and is temporary.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • Fix atime update logic in overlayfs.

    This patch adds an i_op->update_time() handler to overlayfs inodes. This
    forwards atime updates to the upper layer only. No atime updates are done
    on lower layers.

    Remove implicit atime updates to underlying files and directories with
    O_NOATIME. Remove explicit atime update in ovl_readlink().

    Clear atime related mnt flags from cloned upper mount. This means atime
    updates are controlled purely by overlayfs mount options.

    Reported-by: Konstantin Khlebnikov
    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • The fact that we always do permission checking on the overlay inode and
    clear MAY_WRITE for checking access to the lower inode allows cruft to be
    removed from ovl_permission().

    1) "default_permissions" option effectively did generic_permission() on the
    overlay inode with i_mode, i_uid and i_gid updated from underlying
    filesystem. This is what we do by default now. It did the update using
    vfs_getattr() but that's only needed if the underlying filesystem can
    change (which is not allowed). We may later introduce a "paranoia_mode"
    that verifies that mode/uid/gid are not changed.

    2) splitting out the IS_RDONLY() check from inode_permission() also becomes
    unnecessary once we remove the MAY_WRITE from the lower inode check.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     
  • Now we have two levels of checks in ovl_permission(). overlay inode
    is checked with the creds of task while underlying inode is checked
    with the creds of mounter.

    Looks like mounter does not have to have WRITE access to files on lower/.
    So remove the MAY_WRITE from access mask for checks on underlying
    lower inode.

    This means task should still have the MAY_WRITE permission on lower
    inode and mounter is not required to have MAY_WRITE.

    It also solves the problem of read only NFS mounts being used as lower.
    If __inode_permission(lower_inode, MAY_WRITE) is called on read only
    NFS, it fails. By resetting MAY_WRITE, check succeeds and case of
    read only NFS shold work with overlay without having to specify any
    special mount options (default permission).

    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Given we are now doing checks both on overlay inode as well underlying
    inode, we should be able to do checks and operations on underlying file
    system using mounter's context.

    So modify all operations to do checks/operations on underlying dentry/inode
    in the context of mounter.

    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Right now ovl_permission() calls __inode_permission(realinode), to do
    permission checks on real inode and no checks are done on overlay inode.

    Modify it to do checks both on overlay inode as well as underlying inode.
    Checks on overlay inode will be done with the creds of calling task while
    checks on underlying inode will be done with the creds of mounter.

    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Now we are planning to do DAC permission checks on overlay inode
    itself. And to make it work, we will need to make sure we can get acls from
    underlying inode. So define ->get_acl() for overlay inodes and this in turn
    calls into underlying filesystem to get acls, if any.

    Signed-off-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi

    Vivek Goyal
     
  • Previously this was only done for directory inodes. Doing so for all
    inodes makes for a nice cleanup in ovl_permission at zero cost.

    Inodes are not shared for hard links on the overlay, so this works fine.

    Signed-off-by: Miklos Szeredi

    Andreas Gruenbacher
     

27 Jul, 2016

1 commit


04 Jul, 2016

2 commits

  • Right now when a new overlay inode is created, we initialize overlay
    inode's ->i_mode from underlying inode ->i_mode but we retain only
    file type bits (S_IFMT) and discard permission bits.

    This patch changes it and retains permission bits too. This should allow
    overlay to do permission checks on overlay inode itself in task context.

    [SzM] It also fixes clearing suid/sgid bits on write.

    Signed-off-by: Vivek Goyal
    Reported-by: Eryu Guan
    Signed-off-by: Miklos Szeredi
    Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay and f_inode to the underlay")
    Cc:

    Vivek Goyal
     
  • Before 4bacc9c9234c ("overlayfs: Make f_path...") file->f_path pointed to
    the underlying file, hence suid/sgid removal on write worked fine.

    After that patch file->f_path pointed to the overlay file, and the file
    mode bits weren't copied to overlay_inode->i_mode. So the suid/sgid
    removal simply stopped working.

    The fix is to copy the mode bits, but then ovl_setattr() needs to clear
    ATTR_MODE to avoid the BUG() in notify_change(). So do this first, then in
    the next patch copy the mode.

    Reported-by: Eryu Guan
    Signed-off-by: Miklos Szeredi
    Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay and f_inode to the underlay")
    Cc:

    Miklos Szeredi
     

30 Jun, 2016

1 commit

  • The two methods essentially do the same: find the real dentry/inode
    belonging to an overlay dentry. The difference is in the usage:

    vfs_open() uses ->d_select_inode() and expects the function to perform
    copy-up if necessary based on the open flags argument.

    file_dentry() uses ->d_real() passing in the overlay dentry as well as the
    underlying inode.

    vfs_rename() uses ->d_select_inode() but passes zero flags. ->d_real()
    with a zero inode would have worked just as well here.

    This patch merges the functionality of ->d_select_inode() into ->d_real()
    by adding an 'open_flags' argument to the latter.

    [Al Viro] Make the signature of d_real() match that of ->d_real() again.
    And constify the inode argument, while we are at it.

    Signed-off-by: Miklos Szeredi

    Miklos Szeredi
     

29 Jun, 2016

2 commits

  • When truncating a file we should check write access on the underlying
    inode. And we should do so on the lower file as well (before copy-up) for
    consistency.

    Original patch and test case by Aihua Zhang.

    - - >o >o - - test.c - - >o >o - -
    #include
    #include
    #include

    int main(int argc, char *argv[])
    {
    int ret;

    ret = truncate(argv[0], 4096);
    if (ret != -1) {
    fprintf(stderr, "truncate(argv[0]) should have failed\n");
    return 1;
    }
    if (errno != ETXTBSY) {
    perror("truncate(argv[0])");
    return 1;
    }

    return 0;
    }
    - - >o >o - - >o >o - - >o >o - -

    Reported-by: Aihua Zhang
    Signed-off-by: Miklos Szeredi
    Cc:

    Miklos Szeredi
     
  • When using the 'default_permissions' mount option, ovl_permission() on
    non-directories was missing a dput(alias), resulting in "BUG Dentry still
    in use".

    Signed-off-by: Miklos Szeredi
    Fixes: 8d3095f4ad47 ("ovl: default permissions")
    Cc: # v4.5+

    Miklos Szeredi
     

06 Jun, 2016

1 commit

  • a) ovl_need_xattr_filter() is wrong, we can have multiple lower layers
    overlaid, all of which (except the lowest one) honouring the
    "trusted.overlay.opaque" xattr. So need to filter everything except the
    bottom and the pure-upper layer.

    b) we no longer can assume that inode is attached to dentry in
    get/setxattr.

    This patch unconditionally filters private xattrs to fix both of the above.
    Performance impact for get/removexattrs is likely in the noise.

    For listxattrs it might be measurable in pathological cases, but I very
    much hope nobody cares. If they do, we'll fix it then.

    Reported-by: Vivek Goyal
    Signed-off-by: Miklos Szeredi
    Fixes: b96809173e94 ("security_d_instantiate(): move to the point prior to attaching dentry to inode")

    Miklos Szeredi
     

28 May, 2016

1 commit

  • smack ->d_instantiate() uses ->setxattr(), so to be able to call it before
    we'd hashed the new dentry and attached it to inode, we need ->setxattr()
    instances getting the inode as an explicit argument rather than obtaining
    it from dentry.

    Similar change for ->getxattr() had been done in commit ce23e64. Unlike
    ->getxattr() (which is used by both selinux and smack instances of
    ->d_instantiate()) ->setxattr() is used only by smack one and unfortunately
    it got missed back then.

    Reported-by: Seung-Woo Kim
    Tested-by: Casey Schaufler
    Signed-off-by: Al Viro

    Al Viro
     

11 Apr, 2016

1 commit


04 Mar, 2016

1 commit


23 Jan, 2016

1 commit

  • parallel to mutex_{lock,unlock,trylock,is_locked,lock_nested},
    inode_foo(inode) being mutex_foo(&inode->i_mutex).

    Please, use those for access to ->i_mutex; over the coming cycle
    ->i_mutex will become rwsem, with ->lookup() done with it held
    only shared.

    Signed-off-by: Al Viro

    Al Viro