diff mbox

[REVIEW,1/6] vfs: Don't allow changing the link count of an inode with an invalid uid or gid

Message ID 20180523232538.4880-1-ebiederm@xmission.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric W. Biederman May 23, 2018, 11:25 p.m. UTC
Changing the link count of an inode via unlink or link will cause a
write back of that inode.  If the uids or gids are invalid (aka not known
to the kernel) writing the inode back may change the uid or gid in the
filesystem.   To prevent possible filesystem and to avoid the need for
filesystem maintainers to worry about it don't allow operations on
inodes with an invalid uid or gid.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/namei.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Seth Forshee May 24, 2018, 12:58 p.m. UTC | #1
On Wed, May 23, 2018 at 06:25:33PM -0500, Eric W. Biederman wrote:
> Changing the link count of an inode via unlink or link will cause a
> write back of that inode.  If the uids or gids are invalid (aka not known
> to the kernel) writing the inode back may change the uid or gid in the
> filesystem.   To prevent possible filesystem and to avoid the need for
> filesystem maintainers to worry about it don't allow operations on
> inodes with an invalid uid or gid.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Acked-by: Seth Forshee <seth.forshee@canonical.com>
Christian Brauner May 24, 2018, 10:30 p.m. UTC | #2
On Thu, May 24, 2018 at 07:58:32AM -0500, Seth Forshee wrote:
> On Wed, May 23, 2018 at 06:25:33PM -0500, Eric W. Biederman wrote:
> > Changing the link count of an inode via unlink or link will cause a
> > write back of that inode.  If the uids or gids are invalid (aka not known
> > to the kernel) writing the inode back may change the uid or gid in the
> > filesystem.   To prevent possible filesystem and to avoid the need for
> > filesystem maintainers to worry about it don't allow operations on
> > inodes with an invalid uid or gid.
> > 
> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Acked-by: Seth Forshee <seth.forshee@canonical.com>

Acked-by: Christian Brauner <christian@brauner.io>
diff mbox

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 186bd2464fd5..942c1f096f6b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -984,13 +984,15 @@  static bool safe_hardlink_source(struct inode *inode)
  */
 static int may_linkat(struct path *link)
 {
-	struct inode *inode;
+	struct inode *inode = link->dentry->d_inode;
+
+	/* Inode writeback is not safe when the uid or gid are invalid. */
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EOVERFLOW;
 
 	if (!sysctl_protected_hardlinks)
 		return 0;
 
-	inode = link->dentry->d_inode;
-
 	/* Source inode owner (or CAP_FOWNER) can hardlink all they like,
 	 * otherwise, it must be a safe source.
 	 */
@@ -2749,6 +2751,11 @@  static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
 	BUG_ON(!inode);
 
 	BUG_ON(victim->d_parent->d_inode != dir);
+
+	/* Inode writeback is not safe when the uid or gid are invalid. */
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EOVERFLOW;
+
 	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
 
 	error = inode_permission(dir, MAY_WRITE | MAY_EXEC);