mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeongjun Park <aha310510@gmail.com>
To: tytso@mit.edu, adilger.kernel@dilger.ca
Cc: akpm@osdl.org, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jeongjun Park <aha310510@gmail.com>
Subject: [PATCH v2] ext4: prevent data-race that occur when read/write ext4_group_desc structure members
Date: Sat, 21 Sep 2024 00:00:13 +0900	[thread overview]
Message-ID: <20240920150013.2447-1-aha310510@gmail.com> (raw)

Currently, data-race like [1] occur in fs/ext4/ialloc.c

find_group_other() and find_group_orlov() read multiple ext4_groups but
do not protect them with locks, which causes data-race. I think it would
be appropriate to add ext4_lock_group() at an appropriate location to solve
this.

[1]

==================================================================
BUG: KCSAN: data-race in ext4_free_inodes_count / ext4_free_inodes_set

write to 0xffff88810404300e of 2 bytes by task 6254 on cpu 1:
 ext4_free_inodes_set+0x1f/0x80 fs/ext4/super.c:405
 __ext4_new_inode+0x15ca/0x2200 fs/ext4/ialloc.c:1216
 ext4_symlink+0x242/0x5a0 fs/ext4/namei.c:3391
 vfs_symlink+0xca/0x1d0 fs/namei.c:4615
 do_symlinkat+0xe3/0x340 fs/namei.c:4641
 __do_sys_symlinkat fs/namei.c:4657 [inline]
 __se_sys_symlinkat fs/namei.c:4654 [inline]
 __x64_sys_symlinkat+0x5e/0x70 fs/namei.c:4654
 x64_sys_call+0x1dda/0x2d60 arch/x86/include/generated/asm/syscalls_64.h:267
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

read to 0xffff88810404300e of 2 bytes by task 6257 on cpu 0:
 ext4_free_inodes_count+0x1c/0x80 fs/ext4/super.c:349
 find_group_other fs/ext4/ialloc.c:594 [inline]
 __ext4_new_inode+0x6ec/0x2200 fs/ext4/ialloc.c:1017
 ext4_symlink+0x242/0x5a0 fs/ext4/namei.c:3391
 vfs_symlink+0xca/0x1d0 fs/namei.c:4615
 do_symlinkat+0xe3/0x340 fs/namei.c:4641
 __do_sys_symlinkat fs/namei.c:4657 [inline]
 __se_sys_symlinkat fs/namei.c:4654 [inline]
 __x64_sys_symlinkat+0x5e/0x70 fs/namei.c:4654
 x64_sys_call+0x1dda/0x2d60 arch/x86/include/generated/asm/syscalls_64.h:267
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x54/0x120 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

value changed: 0x185c -> 0x185b

Fixes: ac27a0ec112a ("[PATCH] ext4: initial copy of files from ext3")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 fs/ext4/ialloc.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 9dfd768ed9f8..5cae247ff21f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -500,11 +500,14 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
 		for (i = 0; i < flex_size; i++) {
 			if (grp+i >= real_ngroups)
 				break;
+			ext4_lock_group(sb, grp+i);
 			desc = ext4_get_group_desc(sb, grp+i, NULL);
 			if (desc && ext4_free_inodes_count(sb, desc)) {
 				*group = grp+i;
+				ext4_unlock_group(sb, grp+i);
 				return 0;
 			}
+			ext4_unlock_group(sb, grp+i);
 		}
 		goto fallback;
 	}
@@ -544,14 +547,17 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
 	parent_group = EXT4_I(parent)->i_block_group;
 	for (i = 0; i < ngroups; i++) {
 		grp = (parent_group + i) % ngroups;
+		ext4_lock_group(sb, grp);
 		desc = ext4_get_group_desc(sb, grp, NULL);
 		if (desc) {
 			grp_free = ext4_free_inodes_count(sb, desc);
 			if (grp_free && grp_free >= avefreei) {
 				*group = grp;
+				ext4_unlock_group(sb, grp);
 				return 0;
 			}
 		}
+		ext4_unlock_group(sb, grp);
 	}
 
 	if (avefreei) {
@@ -590,11 +596,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
 		if (last > ngroups)
 			last = ngroups;
 		for  (i = parent_group; i < last; i++) {
+			ext4_lock_group(sb, i);
 			desc = ext4_get_group_desc(sb, i, NULL);
 			if (desc && ext4_free_inodes_count(sb, desc)) {
 				*group = i;
+				ext4_unlock_group(sb, i);
 				return 0;
 			}
+			ext4_unlock_group(sb, i);
 		}
 		if (!retry && EXT4_I(parent)->i_last_alloc_group != ~0) {
 			retry = 1;
@@ -616,10 +625,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
 	 * Try to place the inode in its parent directory
 	 */
 	*group = parent_group;
+	ext4_lock_group(sb, *group);
 	desc = ext4_get_group_desc(sb, *group, NULL);
 	if (desc && ext4_free_inodes_count(sb, desc) &&
-	    ext4_free_group_clusters(sb, desc))
+	    ext4_free_group_clusters(sb, desc)) {
+		ext4_unlock_group(sb, *group);
 		return 0;
+	}
+	ext4_unlock_group(sb, *group);
 
 	/*
 	 * We're going to place this inode in a different blockgroup from its
@@ -640,10 +653,14 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
 		*group += i;
 		if (*group >= ngroups)
 			*group -= ngroups;
+		ext4_lock_group(sb, *group);
 		desc = ext4_get_group_desc(sb, *group, NULL);
 		if (desc && ext4_free_inodes_count(sb, desc) &&
-		    ext4_free_group_clusters(sb, desc))
+		    ext4_free_group_clusters(sb, desc)) {
+			ext4_unlock_group(sb, *group);
 			return 0;
+		}
+		ext4_unlock_group(sb, *group);
 	}
 
 	/*
@@ -654,9 +671,13 @@ static int find_group_other(struct super_block *sb, struct inode *parent,
 	for (i = 0; i < ngroups; i++) {
 		if (++*group >= ngroups)
 			*group = 0;
+		ext4_lock_group(sb, *group);
 		desc = ext4_get_group_desc(sb, *group, NULL);
-		if (desc && ext4_free_inodes_count(sb, desc))
+		if (desc && ext4_free_inodes_count(sb, desc)) {
+			ext4_unlock_group(sb, *group);
 			return 0;
+		}
+		ext4_unlock_group(sb, *group);
 	}
 
 	return -1;
--

             reply	other threads:[~2024-09-20 15:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-20 15:00 Jeongjun Park [this message]
2024-09-30 20:54 ` Andreas Dilger
2024-10-01 12:26   ` Jeongjun Park

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=20240920150013.2447-1-aha310510@gmail.com \
    --to=aha310510@gmail.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=akpm@osdl.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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

all inboxes | Powered by JetHome®