lkml.org 
[lkml]   [2015]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.13.y-ckt 85/95] NFS: fix BUG() crash in notify_change() with patch to chown_common()
    Date
    3.13.11-ckt21 -stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Andrew Elble <aweits@rit.edu>

    commit c1b8940b42bb6487b10f2267a96b486276ce9ff7 upstream.

    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.) 6de0ec00ba8d ("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.) 27ac0ffeac80 ("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: 27ac0ffeac80 ("locks: break delegations on any attribute modification")
    Reported-by: Eric Meddaugh <etmsys@rit.edu>
    Tested-by: Eric Meddaugh <etmsys@rit.edu>
    Signed-off-by: Andrew Elble <aweits@rit.edu>
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
    Signed-off-by: Kamal Mostafa <kamal@canonical.com>
    ---
    fs/open.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff --git a/fs/open.c b/fs/open.c
    index 159f3e7..2855a16 100644
    --- a/fs/open.c
    +++ b/fs/open.c
    @@ -539,6 +539,7 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
    uid = make_kuid(current_user_ns(), user);
    gid = make_kgid(current_user_ns(), group);

    +retry_deleg:
    newattrs.ia_valid = ATTR_CTIME;
    if (user != (uid_t) -1) {
    if (!uid_valid(uid))
    @@ -555,7 +556,6 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
    if (!S_ISDIR(inode->i_mode))
    newattrs.ia_valid |=
    ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
    -retry_deleg:
    mutex_lock(&inode->i_mutex);
    error = security_path_chown(path, uid, gid);
    if (!error)
    --
    1.9.1


    \
     
     \ /
      Last update: 2015-05-27 21:41    [W:4.194 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site