linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: Christian Brauner <brauner@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Namjae Jeon <linkinjeon@kernel.org>,
	David Sterba <dsterba@suse.com>,
	David Howells <dhowells@redhat.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Amir Goldstein <amir73il@gmail.com>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Bob Peterson <rpeterso@redhat.com>,
	Steve French <sfrench@samba.org>,
	Luis Chamberlain <mcgrof@kernel.org>
Subject: [PATCH 02/15] exfat: move freeing sbi, upcase table and dropping nls into rcu-delayed helper
Date: Mon, 2 Oct 2023 03:30:15 +0100	[thread overview]
Message-ID: <20231002023015.GC3389589@ZenIV> (raw)
In-Reply-To: <20231002022846.GA3389589@ZenIV>

That stuff can be accessed by ->d_hash()/->d_compare(); as it is, we have
a hard-to-hit UAF if rcu pathwalk manages to get into ->d_hash() on a filesystem
that is in process of getting shut down.

Besides, having nls and upcase table cleanup moved from ->put_super() towards
the place where sbi is freed makes for simpler failure exits.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/exfat/exfat_fs.h |  1 +
 fs/exfat/nls.c      | 14 ++++----------
 fs/exfat/super.c    | 20 +++++++++++---------
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index f55498e5c23d..22e17b0a66e8 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -273,6 +273,7 @@ struct exfat_sb_info {
 
 	spinlock_t inode_hash_lock;
 	struct hlist_head inode_hashtable[EXFAT_HASH_SIZE];
+	struct rcu_head rcu;
 };
 
 #define EXFAT_CACHE_VALID	0
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c
index 705710f93e2d..afdf13c34ff5 100644
--- a/fs/exfat/nls.c
+++ b/fs/exfat/nls.c
@@ -655,7 +655,6 @@ static int exfat_load_upcase_table(struct super_block *sb,
 	unsigned int sect_size = sb->s_blocksize;
 	unsigned int i, index = 0;
 	u32 chksum = 0;
-	int ret;
 	unsigned char skip = false;
 	unsigned short *upcase_table;
 
@@ -673,8 +672,7 @@ static int exfat_load_upcase_table(struct super_block *sb,
 		if (!bh) {
 			exfat_err(sb, "failed to read sector(0x%llx)",
 				  (unsigned long long)sector);
-			ret = -EIO;
-			goto free_table;
+			return -EIO;
 		}
 		sector++;
 		for (i = 0; i < sect_size && index <= 0xFFFF; i += 2) {
@@ -701,15 +699,12 @@ static int exfat_load_upcase_table(struct super_block *sb,
 
 	exfat_err(sb, "failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum : 0x%08x)",
 		  index, chksum, utbl_checksum);
-	ret = -EINVAL;
-free_table:
-	exfat_free_upcase_table(sbi);
-	return ret;
+	return -EINVAL;
 }
 
 static int exfat_load_default_upcase_table(struct super_block *sb)
 {
-	int i, ret = -EIO;
+	int i;
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	unsigned char skip = false;
 	unsigned short uni = 0, *upcase_table;
@@ -740,8 +735,7 @@ static int exfat_load_default_upcase_table(struct super_block *sb)
 		return 0;
 
 	/* FATAL error: default upcase table has error */
-	exfat_free_upcase_table(sbi);
-	return ret;
+	return -EIO;
 }
 
 int exfat_create_upcase_table(struct super_block *sb)
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 2778bd9b631e..593cfff8c6f4 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -39,9 +39,6 @@ static void exfat_put_super(struct super_block *sb)
 	exfat_free_bitmap(sbi);
 	brelse(sbi->boot_bh);
 	mutex_unlock(&sbi->s_lock);
-
-	unload_nls(sbi->nls_io);
-	exfat_free_upcase_table(sbi);
 }
 
 static int exfat_sync_fs(struct super_block *sb, int wait)
@@ -593,7 +590,7 @@ static int __exfat_fill_super(struct super_block *sb)
 	ret = exfat_load_bitmap(sb);
 	if (ret) {
 		exfat_err(sb, "failed to load alloc-bitmap");
-		goto free_upcase_table;
+		goto free_bh;
 	}
 
 	ret = exfat_count_used_clusters(sb, &sbi->used_clusters);
@@ -606,8 +603,6 @@ static int __exfat_fill_super(struct super_block *sb)
 
 free_alloc_bitmap:
 	exfat_free_bitmap(sbi);
-free_upcase_table:
-	exfat_free_upcase_table(sbi);
 free_bh:
 	brelse(sbi->boot_bh);
 	return ret;
@@ -694,12 +689,10 @@ static int exfat_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_root = NULL;
 
 free_table:
-	exfat_free_upcase_table(sbi);
 	exfat_free_bitmap(sbi);
 	brelse(sbi->boot_bh);
 
 check_nls_io:
-	unload_nls(sbi->nls_io);
 	return err;
 }
 
@@ -764,13 +757,22 @@ static int exfat_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void delayed_free(struct rcu_head *p)
+{
+	struct exfat_sb_info *sbi = container_of(p, struct exfat_sb_info, rcu);
+
+	unload_nls(sbi->nls_io);
+	exfat_free_upcase_table(sbi);
+	exfat_free_sbi(sbi);
+}
+
 static void exfat_kill_sb(struct super_block *sb)
 {
 	struct exfat_sb_info *sbi = sb->s_fs_info;
 
 	kill_block_super(sb);
 	if (sbi)
-		exfat_free_sbi(sbi);
+		call_rcu(&sbi->rcu, delayed_free);
 }
 
 static struct file_system_type exfat_fs_type = {
-- 
2.39.2


  parent reply	other threads:[~2023-10-02  2:30 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-02  2:28 [RFC][PATCHES] fixes in methods exposed to rcu pathwalk Al Viro
2023-10-02  2:28 ` Al Viro
2023-10-02  2:29   ` [PATCH 01/15] rcu pathwalk: prevent bogus hard errors from may_lookup() Al Viro
2023-10-02  2:30   ` Al Viro [this message]
2023-10-02 16:10     ` [PATCH 02/15] exfat: move freeing sbi, upcase table and dropping nls into rcu-delayed helper Linus Torvalds
2023-10-02 18:04       ` Al Viro
2023-10-02  2:30   ` [PATCH 03/15] affs: free affs_sb_info with kfree_rcu() Al Viro
2023-10-02  2:31   ` [PATCH 04/15] hfsplus: switch to rcu-delayed unloading of nls and freeing ->s_fs_info Al Viro
2023-10-02  6:49     ` Christoph Hellwig
2023-10-02  7:14       ` Al Viro
2023-10-02  7:21         ` Al Viro
2023-10-02 18:09           ` Al Viro
2023-10-04 19:04             ` Linus Torvalds
2023-10-04 19:06               ` Linus Torvalds
2023-10-02  2:31   ` [PATCH 05/15] cifs_get_link(): bail out in unsafe case Al Viro
2023-10-02  2:32   ` [PATCH 06/15] procfs: move dropping pde and pid from ->evict_inode() to ->free_inode() Al Viro
2023-10-02  2:33   ` [PATCH 07/15] procfs: make freeing proc_fs_info rcu-delayed Al Viro
2023-10-02  2:33   ` [PATCH 08/15] gfs2: fix an oops in gfs2_permission() Al Viro
2023-10-02 11:46     ` Bob Peterson
2023-10-02 12:59       ` Al Viro
2023-10-02 14:16         ` Al Viro
2023-10-03 14:46           ` Andreas Grünbacher
2023-10-02  2:34   ` [PATCH 09/15] nfs: make nfs_set_verifier() safe for use in RCU pathwalk Al Viro
2023-10-02  2:34   ` [PATCH 10/15] nfs: fix UAF on pathwalk running into umount Al Viro
2023-10-02  2:35   ` [PATCH 11/15] fuse: fix UAF in rcu pathwalks Al Viro
2023-10-02  2:35   ` [PATCH 12/15] afs: fix __afs_break_callback() / afs_drop_open_mmap() race Al Viro
2023-10-02  2:36   ` [PATCH 13/15] overlayfs: move freeing ovl_entry past rcu delay Al Viro
2023-10-02  2:36     ` [PATCH 14/15] ovl_dentry_revalidate_common(): fetch inode once Al Viro
2023-10-02  2:37       ` [PATCH 15/15] overlayfs: make use of ->layers safe in rcu pathwalk Al Viro
2023-10-02  6:40         ` Amir Goldstein
2023-10-02  7:23           ` Al Viro
2023-10-02  8:53             ` Amir Goldstein
2023-10-03 20:47               ` Al Viro
2023-10-02  5:47       ` [PATCH 14/15] ovl_dentry_revalidate_common(): fetch inode once Amir Goldstein
2023-10-02  5:56         ` Amir Goldstein
2023-10-02 14:47           ` Amir Goldstein
2023-10-02  5:51     ` [PATCH 13/15] overlayfs: move freeing ovl_entry past rcu delay Amir Goldstein
2023-10-02  2:52   ` [RFC][PATCHES] fixes in methods exposed to rcu pathwalk Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231002023015.GC3389589@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=dsterba@suse.com \
    --cc=hch@lst.de \
    --cc=linkinjeon@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=rpeterso@redhat.com \
    --cc=sfrench@samba.org \
    --cc=torvalds@linux-foundation.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).