Blame SOURCES/CVE-2021-4083.patch

a37bff
From 80c7e812dffa734599aadde93cb8e30b34f0983d Mon Sep 17 00:00:00 2001
a37bff
From: Joe Lawrence <joe.lawrence@redhat.com>
a37bff
Date: Mon, 21 Mar 2022 15:45:03 -0400
a37bff
Subject: [KPATCH CVE-2021-4083] fget: kpatch fixes for CVE-2021-4083
a37bff
a37bff
Kernels:
a37bff
3.10.0-1160.24.1.el7
a37bff
3.10.0-1160.25.1.el7
a37bff
3.10.0-1160.31.1.el7
a37bff
3.10.0-1160.36.2.el7
a37bff
3.10.0-1160.41.1.el7
a37bff
3.10.0-1160.42.2.el7
a37bff
3.10.0-1160.45.1.el7
a37bff
3.10.0-1160.49.1.el7
a37bff
3.10.0-1160.53.1.el7
a37bff
3.10.0-1160.59.1.el7
a37bff
a37bff
Changes since last build:
a37bff
arches: x86_64 ppc64le
a37bff
file.o: changed function: SyS_dup
a37bff
file.o: changed function: dup_fd
a37bff
file.o: changed function: fget
a37bff
file.o: changed function: fget_light
a37bff
file.o: changed function: fget_raw
a37bff
file.o: changed function: fget_raw_light
a37bff
file.o: changed function: put_files_struct
a37bff
file.o: new function: __fget
a37bff
file.o: new function: __fget_light
a37bff
---------------------------
a37bff
a37bff
Kpatch-MR: https://gitlab.com/redhat/prdsc/rhel/src/kpatch/rhel-7/-/merge_requests/34
a37bff
Approved-by: Yannick Cote (@ycote1)
a37bff
Modifications:
a37bff
- include/linux/rcupdate.h, kernel/rcupdate.c: leave exported
a37bff
  rcu_my_thread_group_empty() intact
a37bff
- fs/file.c: use fput() instead of fputs_many() since we skipped commit
a37bff
  ("fs: add fget_many() and fput_many()")
a37bff
- fs/file.c: use fcheck_files() instead of files_lookup_fd_raw() since
a37bff
  we are skipping subsequent commit ("fget: clarify and improve
a37bff
  __fget_files() implementation") that provides it.
a37bff
- Set __attribute__((optimize("-fno-optimize-sibling-calls"))) for
a37bff
  fget() and fget_raw() on ppc64le
a37bff
a37bff
commit c2207a235113315ad696b06eb96ccd36d1f5fdeb
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:29 2022 +0100
a37bff
a37bff
    introduce __fcheck_files() to fix rcu_dereference_check_fdtable(), kill rcu_my_thread_group_empty()
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
    Conflicts:
a37bff
            - context difference due to backport of later patch
a37bff
            - target file difference due to missing backport of rcu source code
a37bff
              move
a37bff
a37bff
    commit a8d4b8345e0ee48b732126d980efaf0dc373e2b0
a37bff
    Author: Oleg Nesterov <oleg@redhat.com>
a37bff
    Date:   Sat Jan 11 19:19:32 2014 +0100
a37bff
a37bff
        introduce __fcheck_files() to fix rcu_dereference_check_fdtable(), kill rcu_my_thread_group_empty()
a37bff
a37bff
        rcu_dereference_check_fdtable() looks very wrong,
a37bff
a37bff
        1. rcu_my_thread_group_empty() was added by 844b9a8707f1 "vfs: fix
a37bff
           RCU-lockdep false positive due to /proc" but it doesn't really
a37bff
           fix the problem. A CLONE_THREAD (without CLONE_FILES) task can
a37bff
           hit the same race with get_files_struct().
a37bff
a37bff
           And otoh rcu_my_thread_group_empty() can suppress the correct
a37bff
           warning if the caller is the CLONE_FILES (without CLONE_THREAD)
a37bff
           task.
a37bff
a37bff
        2. files->count == 1 check is not really right too. Even if this
a37bff
           files_struct is not shared it is not safe to access it lockless
a37bff
           unless the caller is the owner.
a37bff
a37bff
           Otoh, this check is sub-optimal. files->count == 0 always means
a37bff
           it is safe to use it lockless even if files != current->files,
a37bff
           but put_files_struct() has to take rcu_read_lock(). See the next
a37bff
           patch.
a37bff
a37bff
        This patch removes the buggy checks and turns fcheck_files() into
a37bff
        __fcheck_files() which uses rcu_dereference_raw(), the "unshared"
a37bff
        callers, fget_light() and fget_raw_light(), can use it to avoid
a37bff
        the warning from RCU-lockdep.
a37bff
a37bff
        fcheck_files() is trivially reimplemented as rcu_lockdep_assert()
a37bff
        plus __fcheck_files().
a37bff
a37bff
        Signed-off-by: Oleg Nesterov <oleg@redhat.com>
a37bff
        Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
commit ec06bac02991edcfdeb148ab2fe7f3e2d7d3ceaa
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:30 2022 +0100
a37bff
a37bff
    fs: factor out common code in fget() and fget_raw()
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
    Conflicts:
a37bff
            - difference due to backport of later patch
a37bff
a37bff
    commit 1deb46e2562561255c34075825fd00f22a858bb3
a37bff
    Author: Oleg Nesterov <oleg@redhat.com>
a37bff
    Date:   Mon Jan 13 16:48:19 2014 +0100
a37bff
a37bff
        fs: factor out common code in fget() and fget_raw()
a37bff
a37bff
        Apart from FMODE_PATH check fget() and fget_raw() are identical,
a37bff
        shift the code into the new simple helper, __fget(fd, mask). Saves
a37bff
        160 bytes.
a37bff
a37bff
        Signed-off-by: Oleg Nesterov <oleg@redhat.com>
a37bff
        Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
commit ac43fab520f6836e2a7d3d20dd64d6328233ccbe
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:30 2022 +0100
a37bff
a37bff
    fs: factor out common code in fget_light() and fget_raw_light()
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
a37bff
    commit ad46183445043b562856c60b74db664668fb364b
a37bff
    Author: Oleg Nesterov <oleg@redhat.com>
a37bff
    Date:   Mon Jan 13 16:48:40 2014 +0100
a37bff
a37bff
        fs: factor out common code in fget_light() and fget_raw_light()
a37bff
a37bff
        Apart from FMODE_PATH check fget_light() and fget_raw_light() are
a37bff
        identical, shift the code into the new helper, __fget_light(fd, mask).
a37bff
        Saves 208 bytes.
a37bff
a37bff
        Signed-off-by: Oleg Nesterov <oleg@redhat.com>
a37bff
        Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
commit 9e24c8894f5df488a336f0c848f15a7d2f78d163
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:30 2022 +0100
a37bff
a37bff
    fs: __fget_light() can use __fget() in slow path
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
a37bff
    commit e6ff9a9fa4e05c1c03dec63cdc6a87d6dea02755
a37bff
    Author: Oleg Nesterov <oleg@redhat.com>
a37bff
    Date:   Mon Jan 13 16:49:06 2014 +0100
a37bff
a37bff
        fs: __fget_light() can use __fget() in slow path
a37bff
a37bff
        The slow path in __fget_light() can use __fget() to avoid the
a37bff
        code duplication. Saves 232 bytes.
a37bff
a37bff
        Signed-off-by: Oleg Nesterov <oleg@redhat.com>
a37bff
        Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
commit d63fb584ae2d7d9a1620e23e59072cb6929f3833
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:30 2022 +0100
a37bff
a37bff
    fs/file.c: __fget() and dup2() atomicity rules
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
a37bff
    commit 5ba97d2832f87943c43bb69cb1ef86dbc59df5bc
a37bff
    Author: Eric Dumazet <edumazet@google.com>
a37bff
    Date:   Mon Jun 29 17:10:30 2015 +0200
a37bff
a37bff
        fs/file.c: __fget() and dup2() atomicity rules
a37bff
a37bff
        __fget() does lockless fetch of pointer from the descriptor
a37bff
        table, attempts to grab a reference and treats "it was already
a37bff
        zero" as "it's already gone from the table, we just hadn't
a37bff
        seen the store, let's fail".  Unfortunately, that breaks the
a37bff
        atomicity of dup2() - __fget() might see the old pointer,
a37bff
        notice that it's been already dropped and treat that as
a37bff
        "it's closed".  What we should be getting is either the
a37bff
        old file or new one, depending whether we come before or after
a37bff
        dup2().
a37bff
a37bff
        Dmitry had following test failing sometimes :
a37bff
a37bff
        int fd;
a37bff
        void *Thread(void *x) {
a37bff
          char buf;
a37bff
          int n = read(fd, &buf, 1);
a37bff
          if (n != 1)
a37bff
            exit(printf("read failed: n=%d errno=%d\n", n, errno));
a37bff
          return 0;
a37bff
        }
a37bff
a37bff
        int main()
a37bff
        {
a37bff
          fd = open("/dev/urandom", O_RDONLY);
a37bff
          int fd2 = open("/dev/urandom", O_RDONLY);
a37bff
          if (fd == -1 || fd2 == -1)
a37bff
            exit(printf("open failed\n"));
a37bff
          pthread_t th;
a37bff
          pthread_create(&th, 0, Thread, 0);
a37bff
          if (dup2(fd2, fd) == -1)
a37bff
            exit(printf("dup2 failed\n"));
a37bff
          pthread_join(th, 0);
a37bff
          if (close(fd) == -1)
a37bff
            exit(printf("close failed\n"));
a37bff
          if (close(fd2) == -1)
a37bff
            exit(printf("close failed\n"));
a37bff
          printf("DONE\n");
a37bff
          return 0;
a37bff
        }
a37bff
a37bff
        Signed-off-by: Eric Dumazet <edumazet@google.com>
a37bff
        Reported-by: Dmitry Vyukov <dvyukov@google.com>
a37bff
        Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
commit bc04a10c9303dcd9a6305a0452361537257fa0c1
a37bff
Author: Miklos Szeredi <mszeredi@redhat.com>
a37bff
Date:   Fri Jan 21 10:22:31 2022 +0100
a37bff
a37bff
    fget: check that the fd still exists after getting a ref to it
a37bff
a37bff
    Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2032478
a37bff
    Upstream status: Linus
a37bff
    Testing: xfstests
a37bff
    CVE: CVE-2021-4083
a37bff
a37bff
    commit 054aa8d439b9185d4f5eb9a90282d1ce74772969
a37bff
    Author: Linus Torvalds <torvalds@linux-foundation.org>
a37bff
    Date:   Wed Dec 1 10:06:14 2021 -0800
a37bff
a37bff
        fget: check that the fd still exists after getting a ref to it
a37bff
a37bff
        Jann Horn points out that there is another possible race wrt Unix domain
a37bff
        socket garbage collection, somewhat reminiscent of the one fixed in
a37bff
        commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK").
a37bff
a37bff
        See the extended comment about the garbage collection requirements added
a37bff
        to unix_peek_fds() by that commit for details.
a37bff
a37bff
        The race comes from how we can locklessly look up a file descriptor just
a37bff
        as it is in the process of being closed, and with the right artificial
a37bff
        timing (Jann added a few strategic 'mdelay(500)' calls to do that), the
a37bff
        Unix domain socket garbage collector could see the reference count
a37bff
        decrement of the close() happen before fget() took its reference to the
a37bff
        file and the file was attached onto a new file descriptor.
a37bff
a37bff
        This is all (intentionally) correct on the 'struct file *' side, with
a37bff
        RCU lookups and lockless reference counting very much part of the
a37bff
        design.  Getting that reference count out of order isn't a problem per
a37bff
        se.
a37bff
a37bff
        But the garbage collector can get confused by seeing this situation of
a37bff
        having seen a file not having any remaining external references and then
a37bff
        seeing it being attached to an fd.
a37bff
a37bff
        In commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK") the
a37bff
        fix was to serialize the file descriptor install with the garbage
a37bff
        collector by taking and releasing the unix_gc_lock.
a37bff
a37bff
        That's not really an option here, but since this all happens when we are
a37bff
        in the process of looking up a file descriptor, we can instead simply
a37bff
        just re-check that the file hasn't been closed in the meantime, and just
a37bff
        re-do the lookup if we raced with a concurrent close() of the same file
a37bff
        descriptor.
a37bff
a37bff
        Reported-and-tested-by: Jann Horn <jannh@google.com>
a37bff
        Acked-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
        Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
a37bff
a37bff
    Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
a37bff
a37bff
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
a37bff
---
a37bff
 fs/file.c               | 86 +++++++++++++++--------------------------
a37bff
 include/linux/fdtable.h | 35 ++++++++++-------
a37bff
 2 files changed, 53 insertions(+), 68 deletions(-)
a37bff
a37bff
diff --git a/fs/file.c b/fs/file.c
a37bff
index 44bd634b636a..564d60bf0fda 100644
a37bff
--- a/fs/file.c
a37bff
+++ b/fs/file.c
a37bff
@@ -718,42 +718,43 @@ void do_close_on_exec(struct files_struct *files)
a37bff
 	spin_unlock(&files->file_lock);
a37bff
 }
a37bff
 
a37bff
-struct file *fget(unsigned int fd)
a37bff
+static struct file *__fget(unsigned int fd, fmode_t mask)
a37bff
 {
a37bff
-	struct file *file;
a37bff
 	struct files_struct *files = current->files;
a37bff
+	struct file *file;
a37bff
 
a37bff
 	rcu_read_lock();
a37bff
+loop:
a37bff
 	file = fcheck_files(files, fd);
a37bff
 	if (file) {
a37bff
-		/* File object ref couldn't be taken */
a37bff
-		if (file->f_mode & FMODE_PATH || !get_file_rcu(file))
a37bff
+		/* File object ref couldn't be taken.
a37bff
+		 * dup2() atomicity guarantee is the reason
a37bff
+		 * we loop to catch the new file (or NULL pointer)
a37bff
+		 */
a37bff
+		if (file->f_mode & mask)
a37bff
 			file = NULL;
a37bff
+		else if (!get_file_rcu(file))
a37bff
+			goto loop;
a37bff
+		else if (fcheck_files(files, fd) != file) {
a37bff
+			fput(file);
a37bff
+			goto loop;
a37bff
+		}
a37bff
 	}
a37bff
 	rcu_read_unlock();
a37bff
 
a37bff
 	return file;
a37bff
 }
a37bff
 
a37bff
+__attribute__((optimize("-fno-optimize-sibling-calls"))) struct file *fget(unsigned int fd)
a37bff
+{
a37bff
+	return __fget(fd, FMODE_PATH);
a37bff
+}
a37bff
 EXPORT_SYMBOL(fget);
a37bff
 
a37bff
-struct file *fget_raw(unsigned int fd)
a37bff
+__attribute__((optimize("-fno-optimize-sibling-calls"))) struct file *fget_raw(unsigned int fd)
a37bff
 {
a37bff
-	struct file *file;
a37bff
-	struct files_struct *files = current->files;
a37bff
-
a37bff
-	rcu_read_lock();
a37bff
-	file = fcheck_files(files, fd);
a37bff
-	if (file) {
a37bff
-		/* File object ref couldn't be taken */
a37bff
-		if (!atomic_long_inc_not_zero(&file->f_count))
a37bff
-			file = NULL;
a37bff
-	}
a37bff
-	rcu_read_unlock();
a37bff
-
a37bff
-	return file;
a37bff
+	return __fget(fd, 0);
a37bff
 }
a37bff
-
a37bff
 EXPORT_SYMBOL(fget_raw);
a37bff
 
a37bff
 /*
a37bff
@@ -772,56 +773,33 @@ EXPORT_SYMBOL(fget_raw);
a37bff
  * The fput_needed flag returned by fget_light should be passed to the
a37bff
  * corresponding fput_light.
a37bff
  */
a37bff
-struct file *fget_light(unsigned int fd, int *fput_needed)
a37bff
+struct file *__fget_light(unsigned int fd, fmode_t mask, int *fput_needed)
a37bff
 {
a37bff
-	struct file *file;
a37bff
 	struct files_struct *files = current->files;
a37bff
+	struct file *file;
a37bff
 
a37bff
 	*fput_needed = 0;
a37bff
 	if (atomic_read(&files->count) == 1) {
a37bff
-		file = fcheck_files(files, fd);
a37bff
-		if (file && (file->f_mode & FMODE_PATH))
a37bff
+		file = __fcheck_files(files, fd);
a37bff
+		if (file && (file->f_mode & mask))
a37bff
 			file = NULL;
a37bff
 	} else {
a37bff
-		rcu_read_lock();
a37bff
-		file = fcheck_files(files, fd);
a37bff
-		if (file) {
a37bff
-			if (!(file->f_mode & FMODE_PATH) &&
a37bff
-			    atomic_long_inc_not_zero(&file->f_count))
a37bff
-				*fput_needed = 1;
a37bff
-			else
a37bff
-				/* Didn't get the reference, someone's freed */
a37bff
-				file = NULL;
a37bff
-		}
a37bff
-		rcu_read_unlock();
a37bff
+		file = __fget(fd, mask);
a37bff
+		if (file)
a37bff
+			*fput_needed = 1;
a37bff
 	}
a37bff
 
a37bff
 	return file;
a37bff
 }
a37bff
+struct file *fget_light(unsigned int fd, int *fput_needed)
a37bff
+{
a37bff
+	return __fget_light(fd, FMODE_PATH, fput_needed);
a37bff
+}
a37bff
 EXPORT_SYMBOL(fget_light);
a37bff
 
a37bff
 struct file *fget_raw_light(unsigned int fd, int *fput_needed)
a37bff
 {
a37bff
-	struct file *file;
a37bff
-	struct files_struct *files = current->files;
a37bff
-
a37bff
-	*fput_needed = 0;
a37bff
-	if (atomic_read(&files->count) == 1) {
a37bff
-		file = fcheck_files(files, fd);
a37bff
-	} else {
a37bff
-		rcu_read_lock();
a37bff
-		file = fcheck_files(files, fd);
a37bff
-		if (file) {
a37bff
-			if (atomic_long_inc_not_zero(&file->f_count))
a37bff
-				*fput_needed = 1;
a37bff
-			else
a37bff
-				/* Didn't get the reference, someone's freed */
a37bff
-				file = NULL;
a37bff
-		}
a37bff
-		rcu_read_unlock();
a37bff
-	}
a37bff
-
a37bff
-	return file;
a37bff
+	return __fget_light(fd, 0, fput_needed);
a37bff
 }
a37bff
 
a37bff
 void set_close_on_exec(unsigned int fd, int flag)
a37bff
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
a37bff
index 88d74ca9418f..95bcca7c1a0f 100644
a37bff
--- a/include/linux/fdtable.h
a37bff
+++ b/include/linux/fdtable.h
a37bff
@@ -70,29 +70,36 @@ struct files_struct {
a37bff
 	RH_KABI_EXTEND(wait_queue_head_t resize_wait)
a37bff
 };
a37bff
 
a37bff
-#define rcu_dereference_check_fdtable(files, fdtfd) \
a37bff
-	(rcu_dereference_check((fdtfd), \
a37bff
-			       lockdep_is_held(&(files)->file_lock) || \
a37bff
-			       atomic_read(&(files)->count) == 1 || \
a37bff
-			       rcu_my_thread_group_empty()))
a37bff
-
a37bff
-#define files_fdtable(files) \
a37bff
-		(rcu_dereference_check_fdtable((files), (files)->fdt))
a37bff
-
a37bff
 struct file_operations;
a37bff
 struct vfsmount;
a37bff
 struct dentry;
a37bff
 
a37bff
-static inline struct file * fcheck_files(struct files_struct *files, unsigned int fd)
a37bff
+#define rcu_dereference_check_fdtable(files, fdtfd) \
a37bff
+	rcu_dereference_check((fdtfd), lockdep_is_held(&(files)->file_lock))
a37bff
+
a37bff
+#define files_fdtable(files) \
a37bff
+	rcu_dereference_check_fdtable((files), (files)->fdt)
a37bff
+
a37bff
+/*
a37bff
+ * The caller must ensure that fd table isn't shared or hold rcu or file lock
a37bff
+ */
a37bff
+static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd)
a37bff
 {
a37bff
-	struct file * file = NULL;
a37bff
-	struct fdtable *fdt = files_fdtable(files);
a37bff
+	struct fdtable *fdt = rcu_dereference_raw(files->fdt);
a37bff
 
a37bff
 	if (fd < fdt->max_fds) {
a37bff
 		fd = array_index_nospec(fd, fdt->max_fds);
a37bff
-		file = rcu_dereference_check_fdtable(files, fdt->fd[fd]);
a37bff
+		return rcu_dereference_raw(fdt->fd[fd]);
a37bff
 	}
a37bff
-	return file;
a37bff
+	return NULL;
a37bff
+}
a37bff
+
a37bff
+static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd)
a37bff
+{
a37bff
+	rcu_lockdep_assert(rcu_read_lock_held() ||
a37bff
+			   lockdep_is_held(&files->file_lock),
a37bff
+			   "suspicious rcu_dereference_check() usage");
a37bff
+	return __fcheck_files(files, fd);
a37bff
 }
a37bff
 
a37bff
 /*
a37bff
-- 
a37bff
2.26.3
a37bff
a37bff