Inter-revision diff: patch 1

Comparing v5 (message) to v3 (message)

--- v5
+++ v3
@@ -1,6 +1,6 @@
 From: Mickaël Salaün <mic@linux.microsoft.com>
 
-Being able to easily change root directories enables to ease some
+Being able to easily change root directories enable to ease some
 development workflow and can be used as a tool to strengthen
 unprivileged security sandboxes.  chroot(2) is not an access-control
 mechanism per se, but it can be used to limit the absolute view of the
@@ -67,29 +67,14 @@
 Cc: Dominik Brodowski <linux@dominikbrodowski.net>
 Cc: Eric W. Biederman <ebiederm@xmission.com>
 Cc: James Morris <jmorris@namei.org>
-Cc: Jann Horn <jannh@google.com>
 Cc: John Johansen <john.johansen@canonical.com>
+Cc: Kees Cook <keescook@chromium.org>
 Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
 Cc: Serge Hallyn <serge@hallyn.com>
 Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
 Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
-Reviewed-by: Kees Cook <keescook@chromium.org>
-Link: https://lore.kernel.org/r/20210316203633.424794-2-mic@digikod.net
+Link: https://lore.kernel.org/r/20210311105242.874506-2-mic@digikod.net
 ---
-
-Changes since v4:
-* Use READ_ONCE(current->fs->users) (found by Jann Horn).
-* Remove ambiguous example in commit description.
-* Add Reviewed-by Kees Cook.
-
-Changes since v3:
-* Move the new permission checks to a dedicated helper
-  current_chroot_allowed() to make the code easier to read and align
-  with user_path_at(), path_permission() and security_path_chroot()
-  calls (suggested by Kees Cook).
-* Remove now useless included file.
-* Extend commit description.
-* Rebase on v5.12-rc3 .
 
 Changes since v2:
 * Replace path_is_under() check with current_chrooted() to gain the same
@@ -99,19 +84,25 @@
 Changes since v1:
 * Replace custom is_path_beneath() with existing path_is_under().
 ---
- fs/open.c | 23 +++++++++++++++++++++--
- 1 file changed, 21 insertions(+), 2 deletions(-)
+ fs/open.c | 13 ++++++++++++-
+ 1 file changed, 12 insertions(+), 1 deletion(-)
 
 diff --git a/fs/open.c b/fs/open.c
-index e53af13b5835..480010a551b2 100644
+index e53af13b5835..1eb086ed324b 100644
 --- a/fs/open.c
 +++ b/fs/open.c
-@@ -532,6 +532,24 @@ SYSCALL_DEFINE1(fchdir, unsigned int, fd)
- 	return error;
- }
+@@ -22,6 +22,7 @@
+ #include <linux/slab.h>
+ #include <linux/uaccess.h>
+ #include <linux/fs.h>
++#include <linux/path.h>
+ #include <linux/personality.h>
+ #include <linux/pagemap.h>
+ #include <linux/syscalls.h>
+@@ -546,8 +547,18 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename)
+ 	if (error)
+ 		goto dput_and_out;
  
-+static inline int current_chroot_allowed(void)
-+{
 +	/*
 +	 * Changing the root directory for the calling task (and its future
 +	 * children) requires that this task has CAP_SYS_CHROOT in its
@@ -120,30 +111,14 @@
 +	 * As for seccomp, checking no_new_privs avoids scenarios where
 +	 * unprivileged tasks can affect the behavior of privileged children.
 +	 */
-+	if (task_no_new_privs(current) && READ_ONCE(current->fs->users) == 1 &&
-+			!current_chrooted())
-+		return 0;
-+	if (ns_capable(current_user_ns(), CAP_SYS_CHROOT))
-+		return 0;
-+	return -EPERM;
-+}
-+
- SYSCALL_DEFINE1(chroot, const char __user *, filename)
- {
- 	struct path path;
-@@ -546,9 +564,10 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename)
- 	if (error)
+ 	error = -EPERM;
+-	if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT))
++	if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT) &&
++			!(task_no_new_privs(current) && current->fs->users == 1
++				&& !current_chrooted()))
  		goto dput_and_out;
- 
--	error = -EPERM;
--	if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT))
-+	error = current_chroot_allowed();
-+	if (error)
- 		goto dput_and_out;
-+
  	error = security_path_chroot(&path);
  	if (error)
- 		goto dput_and_out;
 -- 
 2.30.2
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help