releases/5.10.176/attr-use-consistent-sgid-stripping-checks.patch - pub/scm/linux/kernel/git/stable/stable-queue - Git at Google

 From stable-owner@vger.kernel.org Sat Mar 18 11:16:16 2023
 From: Amir Goldstein <amir73il@gmail.com>
 Date: Sat, 18 Mar 2023 12:15:27 +0200
 Subject: attr: use consistent sgid stripping checks
 To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 Cc: Sasha Levin <sashal@kernel.org>, "Darrick J . Wong" <djwong@kernel.org>, Leah Rumancik <leah.rumancik@gmail.com>, Chandan Babu R <chandan.babu@oracle.com>, Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org
 Message-ID: <20230318101529.1361673-14-amir73il@gmail.com>

 From: Amir Goldstein <amir73il@gmail.com>

 commit ed5a7047d2011cb6b2bf84ceb6680124cc6a7d95 upstream.

 [backported to 5.10.y, prior to idmapped mounts]

 Currently setgid stripping in file_remove_privs()'s should_remove_suid()
 helper is inconsistent with other parts of the vfs. Specifically, it only
 raises ATTR_KILL_SGID if the inode is S_ISGID and S_IXGRP but not if the
 inode isn't in the caller's groups and the caller isn't privileged over the
 inode although we require this already in setattr_prepare() and
 setattr_copy() and so all filesystem implement this requirement implicitly
 because they have to use setattr_{prepare,copy}() anyway.

 But the inconsistency shows up in setgid stripping bugs for overlayfs in
 xfstests (e.g., generic/673, generic/683, generic/685, generic/686,
 generic/687). For example, we test whether suid and setgid stripping works
 correctly when performing various write-like operations as an unprivileged
 user (fallocate, reflink, write, etc.):

 echo "Test 1 - qa_user, non-exec file $verb"
 setup_testfile
 chmod a+rws $junk_file
 commit_and_check "$qa_user" "$verb" 64k 64k

 The test basically creates a file with 6666 permissions. While the file has
 the S_ISUID and S_ISGID bits set it does not have the S_IXGRP set. On a
 regular filesystem like xfs what will happen is:

 sys_fallocate()
 -> vfs_fallocate()
    -> xfs_file_fallocate()
       -> file_modified()
          -> __file_remove_privs()
             -> dentry_needs_remove_privs()
                -> should_remove_suid()
             -> __remove_privs()
                newattrs.ia_valid = ATTR_FORCE | kill;
                -> notify_change()
                   -> setattr_copy()

 In should_remove_suid() we can see that ATTR_KILL_SUID is raised
 unconditionally because the file in the test has S_ISUID set.

 But we also see that ATTR_KILL_SGID won't be set because while the file
 is S_ISGID it is not S_IXGRP (see above) which is a condition for
 ATTR_KILL_SGID being raised.

 So by the time we call notify_change() we have attr->ia_valid set to
 ATTR_KILL_SUID | ATTR_FORCE. Now notify_change() sees that
 ATTR_KILL_SUID is set and does:

 ia_valid = attr->ia_valid |= ATTR_MODE
 attr->ia_mode = (inode->i_mode & ~S_ISUID);

 which means that when we call setattr_copy() later we will definitely
 update inode->i_mode. Note that attr->ia_mode still contains S_ISGID.

 Now we call into the filesystem's ->setattr() inode operation which will
 end up calling setattr_copy(). Since ATTR_MODE is set we will hit:

 if (ia_valid & ATTR_MODE) {
         umode_t mode = attr->ia_mode;
         vfsgid_t vfsgid = i_gid_into_vfsgid(mnt_userns, inode);
         if (!vfsgid_in_group_p(vfsgid) &&
             !capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID))
                 mode &= ~S_ISGID;
         inode->i_mode = mode;
 }

 and since the caller in the test is neither capable nor in the group of the
 inode the S_ISGID bit is stripped.

 But assume the file isn't suid then ATTR_KILL_SUID won't be raised which
 has the consequence that neither the setgid nor the suid bits are stripped
 even though it should be stripped because the inode isn't in the caller's
 groups and the caller isn't privileged over the inode.

 If overlayfs is in the mix things become a bit more complicated and the bug
 shows up more clearly. When e.g., ovl_setattr() is hit from
 ovl_fallocate()'s call to file_remove_privs() then ATTR_KILL_SUID and
 ATTR_KILL_SGID might be raised but because the check in notify_change() is
 questioning the ATTR_KILL_SGID flag again by requiring S_IXGRP for it to be
 stripped the S_ISGID bit isn't removed even though it should be stripped:

 sys_fallocate()
 -> vfs_fallocate()
    -> ovl_fallocate()
       -> file_remove_privs()
          -> dentry_needs_remove_privs()
             -> should_remove_suid()
          -> __remove_privs()
             newattrs.ia_valid = ATTR_FORCE | kill;
             -> notify_change()
                -> ovl_setattr()
                   // TAKE ON MOUNTER'S CREDS
                   -> ovl_do_notify_change()
                      -> notify_change()
                   // GIVE UP MOUNTER'S CREDS
      // TAKE ON MOUNTER'S CREDS
      -> vfs_fallocate()
         -> xfs_file_fallocate()
            -> file_modified()
               -> __file_remove_privs()
                  -> dentry_needs_remove_privs()
                     -> should_remove_suid()
                  -> __remove_privs()
                     newattrs.ia_valid = attr_force | kill;
                     -> notify_change()

 The fix for all of this is to make file_remove_privs()'s
 should_remove_suid() helper to perform the same checks as we already
 require in setattr_prepare() and setattr_copy() and have notify_change()
 not pointlessly requiring S_IXGRP again. It doesn't make any sense in the
 first place because the caller must calculate the flags via
 should_remove_suid() anyway which would raise ATTR_KILL_SGID.

 While we're at it we move should_remove_suid() from inode.c to attr.c
 where it belongs with the rest of the iattr helpers. Especially since it
 returns ATTR_KILL_S{G,U}ID flags. We also rename it to
 setattr_should_drop_suidgid() to better reflect that it indicates both
 setuid and setgid bit removal and also that it returns attr flags.

 Running xfstests with this doesn't report any regressions. We should really
 try and use consistent checks.

 Reviewed-by: Amir Goldstein <amir73il@gmail.com>
 Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
 Signed-off-by: Amir Goldstein <amir73il@gmail.com>
 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 ---
  Documentation/trace/ftrace.rst |    2 +-
  fs/attr.c                      |   31 +++++++++++++++++--------------
  fs/inode.c                     |    2 +-
  fs/ocfs2/file.c                |    4 ++--
  fs/open.c                      |    6 +++---
  include/linux/fs.h             |    2 +-
  6 files changed, 25 insertions(+), 22 deletions(-)

 --- a/Documentation/trace/ftrace.rst
 +++ b/Documentation/trace/ftrace.rst
 @@ -2923,7 +2923,7 @@ Produces::
                bash-1994  [000] ....  4342.324898: ima_get_action <-process_measurement
                bash-1994  [000] ....  4342.324898: ima_match_policy <-ima_get_action
                bash-1994  [000] ....  4342.324899: do_truncate <-do_last
 -              bash-1994  [000] ....  4342.324899: should_remove_suid <-do_truncate
 +              bash-1994  [000] ....  4342.324899: setattr_should_drop_suidgid <-do_truncate
                bash-1994  [000] ....  4342.324899: notify_change <-do_truncate
                bash-1994  [000] ....  4342.324900: current_fs_time <-notify_change
                bash-1994  [000] ....  4342.324900: current_kernel_time <-current_fs_time
 --- a/fs/attr.c
 +++ b/fs/attr.c
 @@ -45,34 +45,37 @@ int setattr_should_drop_sgid(const struc
  	return 0;
  }

 -/*
 - * The logic we want is
 +/**
 + * setattr_should_drop_suidgid - determine whether the set{g,u}id bit needs to
 + *                               be dropped
 + * @inode:	inode to check
 + *
 + * This function determines whether the set{g,u}id bits need to be removed.
 + * If the setuid bit needs to be removed ATTR_KILL_SUID is returned. If the
 + * setgid bit needs to be removed ATTR_KILL_SGID is returned. If both
 + * set{g,u}id bits need to be removed the corresponding mask of both flags is
 + * returned.
   *
 - *	if suid or (sgid and xgrp)
 - *		remove privs
 + * Return: A mask of ATTR_KILL_S{G,U}ID indicating which - if any - setid bits
 + * to remove, 0 otherwise.
   */
 -int should_remove_suid(struct dentry *dentry)
 +int setattr_should_drop_suidgid(struct inode *inode)
  {
 -	umode_t mode = d_inode(dentry)->i_mode;
 +	umode_t mode = inode->i_mode;
  	int kill = 0;

  	/* suid always must be killed */
  	if (unlikely(mode & S_ISUID))
  		kill = ATTR_KILL_SUID;

 -	/*
 -	 * sgid without any exec bits is just a mandatory locking mark; leave
 -	 * it alone.  If some exec bits are set, it's a real sgid; kill it.
 -	 */
 -	if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
 -		kill |= ATTR_KILL_SGID;
 +	kill |= setattr_should_drop_sgid(inode);

  	if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
  		return kill;

  	return 0;
  }
 -EXPORT_SYMBOL(should_remove_suid);
 +EXPORT_SYMBOL(setattr_should_drop_suidgid);

  static bool chown_ok(const struct inode *inode, kuid_t uid)
  {
 @@ -350,7 +353,7 @@ int notify_change(struct dentry * dentry
  		}
  	}
  	if (ia_valid & ATTR_KILL_SGID) {
 -		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 +		if (mode & S_ISGID) {
  			if (!(ia_valid & ATTR_MODE)) {
  				ia_valid = attr->ia_valid |= ATTR_MODE;
  				attr->ia_mode = inode->i_mode;
 --- a/fs/inode.c
 +++ b/fs/inode.c
 @@ -1868,7 +1868,7 @@ int dentry_needs_remove_privs(struct den
  	if (IS_NOSEC(inode))
  		return 0;

 -	mask = should_remove_suid(dentry);
 +	mask = setattr_should_drop_suidgid(inode);
  	ret = security_inode_need_killpriv(dentry);
  	if (ret < 0)
  		return ret;
 --- a/fs/ocfs2/file.c
 +++ b/fs/ocfs2/file.c
 @@ -1994,7 +1994,7 @@ static int __ocfs2_change_file_space(str
  		}
  	}

 -	if (file && should_remove_suid(file->f_path.dentry)) {
 +	if (file && setattr_should_drop_suidgid(file_inode(file))) {
  		ret = __ocfs2_write_remove_suid(inode, di_bh);
  		if (ret) {
  			mlog_errno(ret);
 @@ -2282,7 +2282,7 @@ static int ocfs2_prepare_inode_for_write
  		 * inode. There's also the dinode i_size state which
  		 * can be lost via setattr during extending writes (we
  		 * set inode->i_size at the end of a write. */
 -		if (should_remove_suid(dentry)) {
 +		if (setattr_should_drop_suidgid(inode)) {
  			if (meta_level == 0) {
  				ocfs2_inode_unlock_for_extent_tree(inode,
  								   &di_bh,
 --- a/fs/open.c
 +++ b/fs/open.c
 @@ -665,10 +665,10 @@ retry_deleg:
  		newattrs.ia_valid |= ATTR_GID;
  		newattrs.ia_gid = gid;
  	}
 -	if (!S_ISDIR(inode->i_mode))
 -		newattrs.ia_valid |=
 -			ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
  	inode_lock(inode);
 +	if (!S_ISDIR(inode->i_mode))
 +		newattrs.ia_valid |= ATTR_KILL_SUID | ATTR_KILL_PRIV |
 +				     setattr_should_drop_sgid(inode);
  	error = security_path_chown(path, uid, gid);
  	if (!error)
  		error = notify_change(path->dentry, &newattrs, &delegated_inode);
 --- a/include/linux/fs.h
 +++ b/include/linux/fs.h
 @@ -2960,7 +2960,7 @@ extern void __destroy_inode(struct inode
  extern struct inode *new_inode_pseudo(struct super_block *sb);
  extern struct inode *new_inode(struct super_block *sb);
  extern void free_inode_nonrcu(struct inode *inode);
 -extern int should_remove_suid(struct dentry *);
 +extern int setattr_should_drop_suidgid(struct inode *);
  extern int file_remove_privs(struct file *);

  extern void __insert_inode_hash(struct inode *, unsigned long hashval);
	From stable-owner@vger.kernel.org Sat Mar 18 11:16:16 2023
	From: Amir Goldstein <amir73il@gmail.com>
	Date: Sat, 18 Mar 2023 12:15:27 +0200
	Subject: attr: use consistent sgid stripping checks
	To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	Cc: Sasha Levin <sashal@kernel.org>, "Darrick J . Wong" <djwong@kernel.org>, Leah Rumancik <leah.rumancik@gmail.com>, Chandan Babu R <chandan.babu@oracle.com>, Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, stable@vger.kernel.org
	Message-ID: <20230318101529.1361673-14-amir73il@gmail.com>

	From: Amir Goldstein <amir73il@gmail.com>

	commit ed5a7047d2011cb6b2bf84ceb6680124cc6a7d95 upstream.

	[backported to 5.10.y, prior to idmapped mounts]

	Currently setgid stripping in file_remove_privs()'s should_remove_suid()
	helper is inconsistent with other parts of the vfs. Specifically, it only
	raises ATTR_KILL_SGID if the inode is S_ISGID and S_IXGRP but not if the
	inode isn't in the caller's groups and the caller isn't privileged over the
	inode although we require this already in setattr_prepare() and
	setattr_copy() and so all filesystem implement this requirement implicitly
	because they have to use setattr_{prepare,copy}() anyway.

	But the inconsistency shows up in setgid stripping bugs for overlayfs in
	xfstests (e.g., generic/673, generic/683, generic/685, generic/686,
	generic/687). For example, we test whether suid and setgid stripping works
	correctly when performing various write-like operations as an unprivileged
	user (fallocate, reflink, write, etc.):

	echo "Test 1 - qa_user, non-exec file $verb"
	setup_testfile
	chmod a+rws $junk_file
	commit_and_check "$qa_user" "$verb" 64k 64k

	The test basically creates a file with 6666 permissions. While the file has
	the S_ISUID and S_ISGID bits set it does not have the S_IXGRP set. On a
	regular filesystem like xfs what will happen is:

	sys_fallocate()
	-> vfs_fallocate()
	-> xfs_file_fallocate()
	-> file_modified()
	-> __file_remove_privs()
	-> dentry_needs_remove_privs()
	-> should_remove_suid()
	-> __remove_privs()
	newattrs.ia_valid = ATTR_FORCE \| kill;
	-> notify_change()
	-> setattr_copy()

	In should_remove_suid() we can see that ATTR_KILL_SUID is raised
	unconditionally because the file in the test has S_ISUID set.

	But we also see that ATTR_KILL_SGID won't be set because while the file
	is S_ISGID it is not S_IXGRP (see above) which is a condition for
	ATTR_KILL_SGID being raised.

	So by the time we call notify_change() we have attr->ia_valid set to
	ATTR_KILL_SUID \| ATTR_FORCE. Now notify_change() sees that
	ATTR_KILL_SUID is set and does:

	ia_valid = attr->ia_valid \|= ATTR_MODE
	attr->ia_mode = (inode->i_mode & ~S_ISUID);

	which means that when we call setattr_copy() later we will definitely
	update inode->i_mode. Note that attr->ia_mode still contains S_ISGID.

	Now we call into the filesystem's ->setattr() inode operation which will
	end up calling setattr_copy(). Since ATTR_MODE is set we will hit:

	if (ia_valid & ATTR_MODE) {
	umode_t mode = attr->ia_mode;
	vfsgid_t vfsgid = i_gid_into_vfsgid(mnt_userns, inode);
	if (!vfsgid_in_group_p(vfsgid) &&
	!capable_wrt_inode_uidgid(mnt_userns, inode, CAP_FSETID))
	mode &= ~S_ISGID;
	inode->i_mode = mode;
	}

	and since the caller in the test is neither capable nor in the group of the
	inode the S_ISGID bit is stripped.

	But assume the file isn't suid then ATTR_KILL_SUID won't be raised which
	has the consequence that neither the setgid nor the suid bits are stripped
	even though it should be stripped because the inode isn't in the caller's
	groups and the caller isn't privileged over the inode.

	If overlayfs is in the mix things become a bit more complicated and the bug
	shows up more clearly. When e.g., ovl_setattr() is hit from
	ovl_fallocate()'s call to file_remove_privs() then ATTR_KILL_SUID and
	ATTR_KILL_SGID might be raised but because the check in notify_change() is
	questioning the ATTR_KILL_SGID flag again by requiring S_IXGRP for it to be
	stripped the S_ISGID bit isn't removed even though it should be stripped:

	sys_fallocate()
	-> vfs_fallocate()
	-> ovl_fallocate()
	-> file_remove_privs()
	-> dentry_needs_remove_privs()
	-> should_remove_suid()
	-> __remove_privs()
	newattrs.ia_valid = ATTR_FORCE \| kill;
	-> notify_change()
	-> ovl_setattr()
	// TAKE ON MOUNTER'S CREDS
	-> ovl_do_notify_change()
	-> notify_change()
	// GIVE UP MOUNTER'S CREDS
	// TAKE ON MOUNTER'S CREDS
	-> vfs_fallocate()
	-> xfs_file_fallocate()
	-> file_modified()
	-> __file_remove_privs()
	-> dentry_needs_remove_privs()
	-> should_remove_suid()
	-> __remove_privs()
	newattrs.ia_valid = attr_force \| kill;
	-> notify_change()

	The fix for all of this is to make file_remove_privs()'s
	should_remove_suid() helper to perform the same checks as we already
	require in setattr_prepare() and setattr_copy() and have notify_change()
	not pointlessly requiring S_IXGRP again. It doesn't make any sense in the
	first place because the caller must calculate the flags via
	should_remove_suid() anyway which would raise ATTR_KILL_SGID.

	While we're at it we move should_remove_suid() from inode.c to attr.c
	where it belongs with the rest of the iattr helpers. Especially since it
	returns ATTR_KILL_S{G,U}ID flags. We also rename it to
	setattr_should_drop_suidgid() to better reflect that it indicates both
	setuid and setgid bit removal and also that it returns attr flags.

	Running xfstests with this doesn't report any regressions. We should really
	try and use consistent checks.

	Reviewed-by: Amir Goldstein <amir73il@gmail.com>
	Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
	Signed-off-by: Amir Goldstein <amir73il@gmail.com>
	Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	---
	Documentation/trace/ftrace.rst \| 2 +-
	fs/attr.c \| 31 +++++++++++++++++--------------
	fs/inode.c \| 2 +-
	fs/ocfs2/file.c \| 4 ++--
	fs/open.c \| 6 +++---
	include/linux/fs.h \| 2 +-
	6 files changed, 25 insertions(+), 22 deletions(-)

	--- a/Documentation/trace/ftrace.rst
	+++ b/Documentation/trace/ftrace.rst
	@@ -2923,7 +2923,7 @@ Produces::
	bash-1994 [000] .... 4342.324898: ima_get_action <-process_measurement
	bash-1994 [000] .... 4342.324898: ima_match_policy <-ima_get_action
	bash-1994 [000] .... 4342.324899: do_truncate <-do_last
	- bash-1994 [000] .... 4342.324899: should_remove_suid <-do_truncate
	+ bash-1994 [000] .... 4342.324899: setattr_should_drop_suidgid <-do_truncate
	bash-1994 [000] .... 4342.324899: notify_change <-do_truncate
	bash-1994 [000] .... 4342.324900: current_fs_time <-notify_change
	bash-1994 [000] .... 4342.324900: current_kernel_time <-current_fs_time
	--- a/fs/attr.c
	+++ b/fs/attr.c
	@@ -45,34 +45,37 @@ int setattr_should_drop_sgid(const struc
	return 0;
	}

	-/*
	- * The logic we want is
	+/**
	+ * setattr_should_drop_suidgid - determine whether the set{g,u}id bit needs to
	+ * be dropped
	+ * @inode: inode to check
	+ *
	+ * This function determines whether the set{g,u}id bits need to be removed.
	+ * If the setuid bit needs to be removed ATTR_KILL_SUID is returned. If the
	+ * setgid bit needs to be removed ATTR_KILL_SGID is returned. If both
	+ * set{g,u}id bits need to be removed the corresponding mask of both flags is
	+ * returned.
	*
	- * if suid or (sgid and xgrp)
	- * remove privs
	+ * Return: A mask of ATTR_KILL_S{G,U}ID indicating which - if any - setid bits
	+ * to remove, 0 otherwise.
	*/
	-int should_remove_suid(struct dentry *dentry)
	+int setattr_should_drop_suidgid(struct inode *inode)
	{
	- umode_t mode = d_inode(dentry)->i_mode;
	+ umode_t mode = inode->i_mode;
	int kill = 0;

	/* suid always must be killed */
	if (unlikely(mode & S_ISUID))
	kill = ATTR_KILL_SUID;

	- /*
	- * sgid without any exec bits is just a mandatory locking mark; leave
	- * it alone. If some exec bits are set, it's a real sgid; kill it.
	- */
	- if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
	- kill \|= ATTR_KILL_SGID;
	+ kill \|= setattr_should_drop_sgid(inode);

	if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
	return kill;

	return 0;
	}
	-EXPORT_SYMBOL(should_remove_suid);
	+EXPORT_SYMBOL(setattr_should_drop_suidgid);

	static bool chown_ok(const struct inode *inode, kuid_t uid)
	{
	@@ -350,7 +353,7 @@ int notify_change(struct dentry * dentry
	}
	}
	if (ia_valid & ATTR_KILL_SGID) {
	- if ((mode & (S_ISGID \| S_IXGRP)) == (S_ISGID \| S_IXGRP)) {
	+ if (mode & S_ISGID) {
	if (!(ia_valid & ATTR_MODE)) {
	ia_valid = attr->ia_valid \|= ATTR_MODE;
	attr->ia_mode = inode->i_mode;
	--- a/fs/inode.c
	+++ b/fs/inode.c
	@@ -1868,7 +1868,7 @@ int dentry_needs_remove_privs(struct den
	if (IS_NOSEC(inode))
	return 0;

	- mask = should_remove_suid(dentry);
	+ mask = setattr_should_drop_suidgid(inode);
	ret = security_inode_need_killpriv(dentry);
	if (ret < 0)
	return ret;
	--- a/fs/ocfs2/file.c
	+++ b/fs/ocfs2/file.c
	@@ -1994,7 +1994,7 @@ static int __ocfs2_change_file_space(str
	}
	}

	- if (file && should_remove_suid(file->f_path.dentry)) {
	+ if (file && setattr_should_drop_suidgid(file_inode(file))) {
	ret = __ocfs2_write_remove_suid(inode, di_bh);
	if (ret) {
	mlog_errno(ret);
	@@ -2282,7 +2282,7 @@ static int ocfs2_prepare_inode_for_write
	* inode. There's also the dinode i_size state which
	* can be lost via setattr during extending writes (we
	* set inode->i_size at the end of a write. */
	- if (should_remove_suid(dentry)) {
	+ if (setattr_should_drop_suidgid(inode)) {
	if (meta_level == 0) {
	ocfs2_inode_unlock_for_extent_tree(inode,
	&di_bh,
	--- a/fs/open.c
	+++ b/fs/open.c
	@@ -665,10 +665,10 @@ retry_deleg:
	newattrs.ia_valid \|= ATTR_GID;
	newattrs.ia_gid = gid;
	}
	- if (!S_ISDIR(inode->i_mode))
	- newattrs.ia_valid \|=
	- ATTR_KILL_SUID \| ATTR_KILL_SGID \| ATTR_KILL_PRIV;
	inode_lock(inode);
	+ if (!S_ISDIR(inode->i_mode))
	+ newattrs.ia_valid \|= ATTR_KILL_SUID \| ATTR_KILL_PRIV \|
	+ setattr_should_drop_sgid(inode);
	error = security_path_chown(path, uid, gid);
	if (!error)
	error = notify_change(path->dentry, &newattrs, &delegated_inode);
	--- a/include/linux/fs.h
	+++ b/include/linux/fs.h
	@@ -2960,7 +2960,7 @@ extern void __destroy_inode(struct inode
	extern struct inode new_inode_pseudo(struct super_block sb);
	extern struct inode new_inode(struct super_block sb);
	extern void free_inode_nonrcu(struct inode *inode);
	-extern int should_remove_suid(struct dentry *);
	+extern int setattr_should_drop_suidgid(struct inode *);
	extern int file_remove_privs(struct file *);

	extern void __insert_inode_hash(struct inode *, unsigned long hashval);