Skip to contentSkip to conversation

userfaultfd: require CAP_SYS_PTRACE for UFFD_FEATURE_EVENT_FORK

PATCH In review [email protected] 1 participant 1 message Dec 1, 2019

Subsystems (2)
Memory management VFS
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

PATCH 149/158 userfaultfd: require CAP_SYS_PTRACE for UFFD_FEATURE_EVENT_FORK

From: Mike Rapoport <[email protected]>
Subject: userfaultfd: require CAP_SYS_PTRACE for UFFD_FEATURE_EVENT_FORK

A while ago Andy noticed
(http://lkml.kernel.org/r/CALCETrWY+5ynDct7eU_nDUqx=okQvjm=Y5wJvA4ahBja=CQXGw@mail.gmail.com)
that UFFD_FEATURE_EVENT_FORK used by an unprivileged user may have security implications.

As the first step of the solution the following patch limits the availably of UFFD_FEATURE_EVENT_FORK only for those having CAP_SYS_PTRACE.

The usage of CAP_SYS_PTRACE ensures compatibility with CRIU.

Yet, if there are other users of non-cooperative userfaultfd that run without CAP_SYS_PTRACE, they would be broken :(

Current implementation of UFFD_FEATURE_EVENT_FORK modifies the file descriptor table from the read() implementation of uffd, which may have security implications for unprivileged use of the userfaultfd.

Limit availability of UFFD_FEATURE_EVENT_FORK only for callers that have CAP_SYS_PTRACE.

Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Mike Rapoport <[email protected]>
Reviewed-by Andrea Arcangeli <[email protected]>
Cc Daniel Colascione <[email protected]>
Cc Jann Horn <[email protected]>
Cc Lokesh Gidra <[email protected]>
Cc Nick Kralevich <[email protected]>
Cc Nosh Minwalla <[email protected]>
Cc Pavel Emelyanov <[email protected]>
Cc Tim Murray <[email protected]>
Cc Aleksa Sarai <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 fs/userfaultfd.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
fs/userfaultfd.c renamed from fs/userfaultfd.c~userfaultfd-require-cap_sys_ptrace-for-uffd_feature_event_fork +11 -7
@@ -1835,13 +1835,12 @@ static int userfaultfd_api(struct userfa
 	if (copy_from_user(&uffdio_api, buf, sizeof(uffdio_api)))
 		goto out;
 	features = uffdio_api.features;
-	if (uffdio_api.api != UFFD_API || (features & ~UFFD_API_FEATURES)) {
-		memset(&uffdio_api, 0, sizeof(uffdio_api));
-		if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api)))
-			goto out;
-		ret = -EINVAL;
-		goto out;
-	}
+	ret = -EINVAL;
+	if (uffdio_api.api != UFFD_API || (features & ~UFFD_API_FEATURES))
+		goto err_out;
+	ret = -EPERM;
+	if ((features & UFFD_FEATURE_EVENT_FORK) && !capable(CAP_SYS_PTRACE))
+		goto err_out;
 	/* report all available features and ioctls to userland */
 	uffdio_api.features = UFFD_API_FEATURES;
 	uffdio_api.ioctls = UFFD_API_IOCTLS;

@@ -1854,6 +1853,11 @@ static int userfaultfd_api(struct userfa
 	ret = 0;
 out:
 	return ret;
+err_out:
+	memset(&uffdio_api, 0, sizeof(uffdio_api));
+	if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api)))
+		ret = -EFAULT;
+	goto out;
 }
 
 static long userfaultfd_ioctl(struct file *file, unsigned cmd,

_