Commit c1b8940b authored by Andrew Elble's avatar Andrew Elble Committed by Al Viro

NFS: fix BUG() crash in notify_change() with patch to chown_common()

We have observed a BUG() crash in fs/attr.c:notify_change(). The crash
occurs during an rsync into a filesystem that is exported via NFS.

1.) fs/attr.c:notify_change() modifies the caller's version of attr.
2.) 6de0ec00 ("VFS: make notify_change pass ATTR_KILL_S*ID to
    setattr operations") introduced a BUG() restriction such that "no
    function will ever call notify_change() with both ATTR_MODE and
    ATTR_KILL_S*ID set". Under some circumstances though, it will have
    assisted in setting the caller's version of attr to this very
    combination.
3.) 27ac0ffe ("locks: break delegations on any attribute
    modification") introduced code to handle breaking
    delegations. This can result in notify_change() being re-called. attr
    _must_ be explicitly reset to avoid triggering the BUG() established
    in #2.
4.) The path that that triggers this is via fs/open.c:chmod_common().
    The combination of attr flags set here and in the first call to
    notify_change() along with a later failed break_deleg_wait()
    results in notify_change() being called again via retry_deleg
    without resetting attr.

Solution is to move retry_deleg in chmod_common() a bit further up to
ensure attr is completely reset.

There are other places where this seemingly could occur, such as
fs/utimes.c:utimes_common(), but the attr flags are not initially
set in such a way to trigger this.

Fixes: 27ac0ffe ("locks: break delegations on any attribute modification")
Reported-by: default avatarEric Meddaugh <etmsys@rit.edu>
Tested-by: default avatarEric Meddaugh <etmsys@rit.edu>
Signed-off-by: default avatarAndrew Elble <aweits@rit.edu>
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 3d330dc1
...@@ -570,6 +570,7 @@ static int chown_common(struct path *path, uid_t user, gid_t group) ...@@ -570,6 +570,7 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
uid = make_kuid(current_user_ns(), user); uid = make_kuid(current_user_ns(), user);
gid = make_kgid(current_user_ns(), group); gid = make_kgid(current_user_ns(), group);
retry_deleg:
newattrs.ia_valid = ATTR_CTIME; newattrs.ia_valid = ATTR_CTIME;
if (user != (uid_t) -1) { if (user != (uid_t) -1) {
if (!uid_valid(uid)) if (!uid_valid(uid))
...@@ -586,7 +587,6 @@ static int chown_common(struct path *path, uid_t user, gid_t group) ...@@ -586,7 +587,6 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
if (!S_ISDIR(inode->i_mode)) if (!S_ISDIR(inode->i_mode))
newattrs.ia_valid |= newattrs.ia_valid |=
ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV; ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
retry_deleg:
mutex_lock(&inode->i_mutex); mutex_lock(&inode->i_mutex);
error = security_path_chown(path, uid, gid); error = security_path_chown(path, uid, gid);
if (!error) if (!error)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment