Skip to content

Commit c1b8940

Browse files
aweitsAl Viro
authored and
Al Viro
committed
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.) 6de0ec0 ("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.) 27ac0ff ("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: 27ac0ff ("locks: break delegations on any attribute modification") Reported-by: Eric Meddaugh <[email protected]> Tested-by: Eric Meddaugh <[email protected]> Signed-off-by: Andrew Elble <[email protected]> Signed-off-by: Al Viro <[email protected]>
1 parent 3d330dc commit c1b8940

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

fs/open.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
570570
uid = make_kuid(current_user_ns(), user);
571571
gid = make_kgid(current_user_ns(), group);
572572

573+
retry_deleg:
573574
newattrs.ia_valid = ATTR_CTIME;
574575
if (user != (uid_t) -1) {
575576
if (!uid_valid(uid))
@@ -586,7 +587,6 @@ static int chown_common(struct path *path, uid_t user, gid_t group)
586587
if (!S_ISDIR(inode->i_mode))
587588
newattrs.ia_valid |=
588589
ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
589-
retry_deleg:
590590
mutex_lock(&inode->i_mutex);
591591
error = security_path_chown(path, uid, gid);
592592
if (!error)

0 commit comments

Comments
 (0)