From patchwork Fri Jan 21 14:26:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1582602 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=K379wv8s; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JgMBw6fKnz9t2p for ; Sat, 22 Jan 2022 01:26:48 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1nAusF-0004zt-Sp; Fri, 21 Jan 2022 14:26:43 +0000 Received: from smtp-relay-canonical-1.internal ([10.131.114.174] helo=smtp-relay-canonical-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1nAusD-0004yb-ON for kernel-team@lists.ubuntu.com; Fri, 21 Jan 2022 14:26:41 +0000 Received: from localhost.localdomain (unknown [179.93.158.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-1.canonical.com (Postfix) with ESMTPSA id D730A3FCE9 for ; Fri, 21 Jan 2022 14:26:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1642775201; bh=dMsYzauwnEscgcxZuaIh6M3FsEXFbJD4phSOHnN8PCk=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=K379wv8sUwgnTV73P2v+Az+vycdjd6/T+fddkBjUhjxhunLiUX4dSTVhz8DUzCO+z 64lOW7+4NGnFFnb6f3QyJWChguamGJDr/0EUd+ahw2zZKU2XiafB4Kp+EAbXJ0IsMl mD3amnBJc18bgTw8X71dYfXY0olbuF+FNrP2rO//s63LcbjFkPNXi0a3EQ1R+vGVf2 1H8UOG7Sq+LsTrhmiGnid2kZNtdQh90nm6Slc3E3UJ/SsjxHl8YvA5t9MF0HbWVJSy 7UUy0gjoOWslFjmw3BiBHvIpG7NT+eFjIXH6fMTtUr10TSEUDBzRvICjt4Le/ezcXu 3HiGgn1B0KAfQ== From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU Bionic 1/2] fs: add fget_many() and fput_many() Date: Fri, 21 Jan 2022 11:26:14 -0300 Message-Id: <20220121142616.163592-2-cascardo@canonical.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20220121142616.163592-1-cascardo@canonical.com> References: <20220121142616.163592-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jens Axboe Some uses cases repeatedly get and put references to the same file, but the only exposed interface is doing these one at the time. As each of these entail an atomic inc or dec on a shared structure, that cost can add up. Add fget_many(), which works just like fget(), except it takes an argument for how many references to get on the file. Ditto fput_many(), which can drop an arbitrary number of references to a file. Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Signed-off-by: Jens Axboe (cherry picked from commit 091141a42e15fe47ada737f3996b317072afcefb) CVE-2021-4083 Signed-off-by: Thadeu Lima de Souza Cascardo --- fs/file.c | 15 ++++++++++----- fs/file_table.c | 9 +++++++-- include/linux/file.h | 2 ++ include/linux/fs.h | 4 +++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/fs/file.c b/fs/file.c index 985a8498afb4..3c4b9cf7ef17 100644 --- a/fs/file.c +++ b/fs/file.c @@ -681,7 +681,7 @@ void do_close_on_exec(struct files_struct *files) spin_unlock(&files->file_lock); } -static struct file *__fget(unsigned int fd, fmode_t mask) +static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) { struct files_struct *files = current->files; struct file *file; @@ -696,7 +696,7 @@ static struct file *__fget(unsigned int fd, fmode_t mask) */ if (file->f_mode & mask) file = NULL; - else if (!get_file_rcu(file)) + else if (!get_file_rcu_many(file, refs)) goto loop; } rcu_read_unlock(); @@ -704,15 +704,20 @@ static struct file *__fget(unsigned int fd, fmode_t mask) return file; } +struct file *fget_many(unsigned int fd, unsigned int refs) +{ + return __fget(fd, FMODE_PATH, refs); +} + struct file *fget(unsigned int fd) { - return __fget(fd, FMODE_PATH); + return __fget(fd, FMODE_PATH, 1); } EXPORT_SYMBOL(fget); struct file *fget_raw(unsigned int fd) { - return __fget(fd, 0); + return __fget(fd, 0, 1); } EXPORT_SYMBOL(fget_raw); @@ -743,7 +748,7 @@ static unsigned long __fget_light(unsigned int fd, fmode_t mask) return 0; return (unsigned long)file; } else { - file = __fget(fd, mask); + file = __fget(fd, mask, 1); if (!file) return 0; return FDPUT_FPUT | (unsigned long)file; diff --git a/fs/file_table.c b/fs/file_table.c index 7bf57df4f5c0..01a9f85cfe42 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -263,9 +263,9 @@ EXPORT_SYMBOL_GPL(flush_delayed_fput); static DECLARE_DELAYED_WORK(delayed_fput_work, delayed_fput); -void fput(struct file *file) +void fput_many(struct file *file, unsigned int refs) { - if (atomic_long_dec_and_test(&file->f_count)) { + if (atomic_long_sub_and_test(refs, &file->f_count)) { struct task_struct *task = current; if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { @@ -284,6 +284,11 @@ void fput(struct file *file) } } +void fput(struct file *file) +{ + fput_many(file, 1); +} + /* * synchronous analog of fput(); for kernel threads that might be needed * in some umount() (and thus can't use flush_delayed_fput() without diff --git a/include/linux/file.h b/include/linux/file.h index 76e38eade225..1f03f8f2bc25 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -13,6 +13,7 @@ struct file; extern void fput(struct file *); +extern void fput_many(struct file *, unsigned int); struct file_operations; struct vfsmount; @@ -42,6 +43,7 @@ static inline void fdput(struct fd fd) } extern struct file *fget(unsigned int fd); +extern struct file *fget_many(unsigned int fd, unsigned int refs); extern struct file *fget_raw(unsigned int fd); extern unsigned long __fdget(unsigned int fd); extern unsigned long __fdget_raw(unsigned int fd); diff --git a/include/linux/fs.h b/include/linux/fs.h index 254c56efd307..cfa85a8b7bb2 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -908,7 +908,9 @@ static inline struct file *get_file(struct file *f) atomic_long_inc(&f->f_count); return f; } -#define get_file_rcu(x) atomic_long_inc_not_zero(&(x)->f_count) +#define get_file_rcu_many(x, cnt) \ + atomic_long_add_unless(&(x)->f_count, (cnt), 0) +#define get_file_rcu(x) get_file_rcu_many((x), 1) #define fput_atomic(x) atomic_long_add_unless(&(x)->f_count, -1, 1) #define file_count(x) atomic_long_read(&(x)->f_count) From patchwork Fri Jan 21 14:26:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1582603 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=tEwebwzk; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JgMBy70YYz9t2p for ; Sat, 22 Jan 2022 01:26:50 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1nAusI-00052n-3E; Fri, 21 Jan 2022 14:26:46 +0000 Received: from smtp-relay-canonical-1.internal ([10.131.114.174] helo=smtp-relay-canonical-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1nAusG-00050i-LQ for kernel-team@lists.ubuntu.com; Fri, 21 Jan 2022 14:26:44 +0000 Received: from localhost.localdomain (unknown [179.93.158.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-1.canonical.com (Postfix) with ESMTPSA id C6F903FCE9 for ; Fri, 21 Jan 2022 14:26:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1642775204; bh=ATJLNNGcj15kwQlQ68dBVMWcMUK8yvuBcPQpThAGYdI=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=tEwebwzkebISVinavOUqWiBK0zcQ+BJi+VKsH4Gaj0OjKQdSKjphJHM3YIRfXPxKW 0w0BYq/P4uRlIDes7uijcqq91LnDALo4biTdDmx+W4Yb0lQCviF+rdNakIt8+r/JVS HuFVjM059ttwhIkzke2gep2hJYcihtl+0YLHb/r5U+0WjL0L+bQYQTSnrwKvHQEsIp bcmZjF3uvlWeeU+BcGcUT7xhwjU6p+7lHbjPi4eO0E1Jv1/o6GeLFpWaBNBNDBIF2q 3zO94WIaMpz+/v70PRjJkXJAH2aQshgzAB5DBGud/ZhIDa72nWtxGbtsKX+hSCq1et jJHG4eyMWXl1w== From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU OEM-5.10, Focal, Bionic 2/2] fget: check that the fd still exists after getting a ref to it Date: Fri, 21 Jan 2022 11:26:15 -0300 Message-Id: <20220121142616.163592-3-cascardo@canonical.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20220121142616.163592-1-cascardo@canonical.com> References: <20220121142616.163592-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Linus Torvalds Jann Horn points out that there is another possible race wrt Unix domain socket garbage collection, somewhat reminiscent of the one fixed in commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK"). See the extended comment about the garbage collection requirements added to unix_peek_fds() by that commit for details. The race comes from how we can locklessly look up a file descriptor just as it is in the process of being closed, and with the right artificial timing (Jann added a few strategic 'mdelay(500)' calls to do that), the Unix domain socket garbage collector could see the reference count decrement of the close() happen before fget() took its reference to the file and the file was attached onto a new file descriptor. This is all (intentionally) correct on the 'struct file *' side, with RCU lookups and lockless reference counting very much part of the design. Getting that reference count out of order isn't a problem per se. But the garbage collector can get confused by seeing this situation of having seen a file not having any remaining external references and then seeing it being attached to an fd. In commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK") the fix was to serialize the file descriptor install with the garbage collector by taking and releasing the unix_gc_lock. That's not really an option here, but since this all happens when we are in the process of looking up a file descriptor, we can instead simply just re-check that the file hasn't been closed in the meantime, and just re-do the lookup if we raced with a concurrent close() of the same file descriptor. Reported-and-tested-by: Jann Horn Acked-by: Miklos Szeredi Signed-off-by: Linus Torvalds (backported from commit 054aa8d439b9185d4f5eb9a90282d1ce74772969) [cascardo: __fcheck_files was renamed to files_lookup_fd_raw at bebf684bf330 ("file: Rename __fcheck_files to files_lookup_fd_raw")] CVE-2021-4083 Signed-off-by: Thadeu Lima de Souza Cascardo --- fs/file.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/file.c b/fs/file.c index 3c4b9cf7ef17..5913a5db9c2a 100644 --- a/fs/file.c +++ b/fs/file.c @@ -698,6 +698,10 @@ static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) file = NULL; else if (!get_file_rcu_many(file, refs)) goto loop; + else if (__fcheck_files(files, fd) != file) { + fput_many(file, refs); + goto loop; + } } rcu_read_unlock();