Skip to content

Commit ff84772

Browse files
Sungjong Seonamjaejeon
authored andcommitted
exfat: release s_lock before calling dir_emit()
There is a potential deadlock reported by syzbot as below: ====================================================== WARNING: possible circular locking dependency detected 6.4.0-next-20230707-syzkaller #0 Not tainted ------------------------------------------------------ syz-executor330/5073 is trying to acquire lock: ffff8880218527a0 (&mm->mmap_lock){++++}-{3:3}, at: mmap_read_lock_killable include/linux/mmap_lock.h:151 [inline] ffff8880218527a0 (&mm->mmap_lock){++++}-{3:3}, at: get_mmap_lock_carefully mm/memory.c:5293 [inline] ffff8880218527a0 (&mm->mmap_lock){++++}-{3:3}, at: lock_mm_and_find_vma+0x369/0x510 mm/memory.c:5344 but task is already holding lock: ffff888019f760e0 (&sbi->s_lock){+.+.}-{3:3}, at: exfat_iterate+0x117/0xb50 fs/exfat/dir.c:232 which lock already depends on the new lock. Chain exists of: &mm->mmap_lock --> mapping.invalidate_lock#3 --> &sbi->s_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&sbi->s_lock); lock(mapping.invalidate_lock#3); lock(&sbi->s_lock); rlock(&mm->mmap_lock); Let's try to avoid above potential deadlock condition by moving dir_emit*() out of sbi->s_lock coverage. Fixes: ca06197 ("exfat: add directory operations") Cc: stable@vger.kernel.org #v5.7+ Reported-by: syzbot+1741a5d9b79989c10bdc@syzkaller.appspotmail.com Link: https://lore.kernel.org/lkml/00000000000078ee7e060066270b@google.com/T/#u Tested-by: syzbot+1741a5d9b79989c10bdc@syzkaller.appspotmail.com Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
1 parent d423345 commit ff84772

File tree

1 file changed

+12
-15
lines changed

1 file changed

+12
-15
lines changed

fs/exfat/dir.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,10 @@ static void exfat_free_namebuf(struct exfat_dentry_namebuf *nb)
218218
exfat_init_namebuf(nb);
219219
}
220220

221-
/* skip iterating emit_dots when dir is empty */
221+
/*
222+
* Before calling dir_emit*(), sbi->s_lock should be released
223+
* because page fault can occur in dir_emit*().
224+
*/
222225
#define ITER_POS_FILLED_DOTS (2)
223226
static int exfat_iterate(struct file *file, struct dir_context *ctx)
224227
{
@@ -233,11 +236,10 @@ static int exfat_iterate(struct file *file, struct dir_context *ctx)
233236
int err = 0, fake_offset = 0;
234237

235238
exfat_init_namebuf(nb);
236-
mutex_lock(&EXFAT_SB(sb)->s_lock);
237239

238240
cpos = ctx->pos;
239241
if (!dir_emit_dots(file, ctx))
240-
goto unlock;
242+
goto out;
241243

242244
if (ctx->pos == ITER_POS_FILLED_DOTS) {
243245
cpos = 0;
@@ -249,16 +251,18 @@ static int exfat_iterate(struct file *file, struct dir_context *ctx)
249251
/* name buffer should be allocated before use */
250252
err = exfat_alloc_namebuf(nb);
251253
if (err)
252-
goto unlock;
254+
goto out;
253255
get_new:
256+
mutex_lock(&EXFAT_SB(sb)->s_lock);
257+
254258
if (ei->flags == ALLOC_NO_FAT_CHAIN && cpos >= i_size_read(inode))
255259
goto end_of_dir;
256260

257261
err = exfat_readdir(inode, &cpos, &de);
258262
if (err) {
259263
/*
260-
* At least we tried to read a sector. Move cpos to next sector
261-
* position (should be aligned).
264+
* At least we tried to read a sector.
265+
* Move cpos to next sector position (should be aligned).
262266
*/
263267
if (err == -EIO) {
264268
cpos += 1 << (sb->s_blocksize_bits);
@@ -281,26 +285,19 @@ static int exfat_iterate(struct file *file, struct dir_context *ctx)
281285
inum = iunique(sb, EXFAT_ROOT_INO);
282286
}
283287

284-
/*
285-
* Before calling dir_emit(), sb_lock should be released.
286-
* Because page fault can occur in dir_emit() when the size
287-
* of buffer given from user is larger than one page size.
288-
*/
289288
mutex_unlock(&EXFAT_SB(sb)->s_lock);
290289
if (!dir_emit(ctx, nb->lfn, strlen(nb->lfn), inum,
291290
(de.attr & ATTR_SUBDIR) ? DT_DIR : DT_REG))
292-
goto out_unlocked;
293-
mutex_lock(&EXFAT_SB(sb)->s_lock);
291+
goto out;
294292
ctx->pos = cpos;
295293
goto get_new;
296294

297295
end_of_dir:
298296
if (!cpos && fake_offset)
299297
cpos = ITER_POS_FILLED_DOTS;
300298
ctx->pos = cpos;
301-
unlock:
302299
mutex_unlock(&EXFAT_SB(sb)->s_lock);
303-
out_unlocked:
300+
out:
304301
/*
305302
* To improve performance, free namebuf after unlock sb_lock.
306303
* If namebuf is not allocated, this function do nothing

0 commit comments

Comments
 (0)