linux-kernel - [PATCH] sysctl: fix permission check while owner isn't GLOBAL_ROOT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210625083338.384184-1-yang.yang29@zte.com.cn>
Date:   Fri, 25 Jun 2021 01:33:38 -0700
From:   menglong8.dong@...il.com
To:     mcgrof@...nel.org
Cc:     keescook@...omium.org, yzaikin@...gle.com,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        Yang Yang <yang.yang29@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: [PATCH] sysctl: fix permission check while owner isn't GLOBAL_ROOT_UID

From: Yang Yang <yang.yang29@....com.cn>

With user namespace enabled, root in container can't modify
/proc/sys/net/ipv4/ip_forward. While /proc/sys/net/ipv4/ip_forward
belongs to root and mode is 644. Since root in container may
be non-root in host, but test_perm() doesn't consider about it.

Reported-by: Zeal Robot <zealci@....com.cn>
Signed-off-by: Yang Yang <yang.yang29@....com.cn>
---
 fs/proc/proc_sysctl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index dea0f5ee540c..71d7b2c2c8e3 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -400,18 +400,19 @@ static void next_entry(struct ctl_table_header **phead, struct ctl_table **pentr
  * some sysctl variables are readonly even to root.
  */
 
-static int test_perm(int mode, int op)
+static int test_perm(struct inode *inode, int mode, int op)
 {
-	if (uid_eq(current_euid(), GLOBAL_ROOT_UID))
+	if (uid_eq(current_euid(), inode->i_uid))
 		mode >>= 6;
-	else if (in_egroup_p(GLOBAL_ROOT_GID))
+	else if (in_egroup_p(inode->i_gid))
 		mode >>= 3;
 	if ((op & ~mode & (MAY_READ|MAY_WRITE|MAY_EXEC)) == 0)
 		return 0;
 	return -EACCES;
 }
 
-static int sysctl_perm(struct ctl_table_header *head, struct ctl_table *table, int op)
+static int sysctl_perm(struct inode *inode,
+	struct ctl_table_header *head, struct ctl_table *table, int op)
 {
 	struct ctl_table_root *root = head->root;
 	int mode;
@@ -421,7 +422,7 @@ static int sysctl_perm(struct ctl_table_header *head, struct ctl_table *table, i
 	else
 		mode = table->mode;
 
-	return test_perm(mode, op);
+	return test_perm(inode, mode, op);
 }
 
 static struct inode *proc_sys_make_inode(struct super_block *sb,
@@ -554,7 +555,7 @@ static ssize_t proc_sys_call_handler(struct kiocb *iocb, struct iov_iter *iter,
 	 * and won't be until we finish.
 	 */
 	error = -EPERM;
-	if (sysctl_perm(head, table, write ? MAY_WRITE : MAY_READ))
+	if (sysctl_perm(inode, head, table, write ? MAY_WRITE : MAY_READ))
 		goto out;
 
 	/* if that can happen at all, it should be -EINVAL, not -EISDIR */
@@ -803,7 +804,7 @@ static int proc_sys_permission(struct user_namespace *mnt_userns,
 	if (!table) /* global root - r-xr-xr-x */
 		error = mask & MAY_WRITE ? -EACCES : 0;
 	else /* Use the permissions on the sysctl table entry */
-		error = sysctl_perm(head, table, mask & ~MAY_NOT_BLOCK);
+		error = sysctl_perm(inode, head, table, mask & ~MAY_NOT_BLOCK);
 
 	sysctl_head_finish(head);
 	return error;
-- 
2.25.1