* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters [not found] <67313d9e.050a0220.138bd5.0054.GAE@google.com> @ 2024-11-11 13:07 ` OGAWA Hirofumi 2024-11-19 7:27 ` [PATCH] loop: Fix ABBA locking race (Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters) OGAWA Hirofumi 2024-11-19 12:10 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters Ming Lei 0 siblings, 2 replies; 7+ messages in thread From: OGAWA Hirofumi @ 2024-11-11 13:07 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs Hi, syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: > syzbot found the following issue on: > > HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 > git tree: linux-next > console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 > kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 > dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc > compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 This patch is to fix the above race. Please check this. Thanks From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Subject: [PATCH] loop: Fix ABBA locking race Date: Mon, 11 Nov 2024 21:53:36 +0900 Current loop calls vfs_statfs() while holding the q->limits_lock. If FS takes some locking in vfs_statfs callback, this may lead to ABBA locking bug (at least, FAT fs has this issue actually). So this patch calls vfs_statfs() outside q->limits_locks instead, because looks like there is no reason to hold q->limits_locks while getting discard configs. Chain exists of: &sbi->fat_lock --> &q->q_usage_counter(io)#17 --> &q->limits_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&q->limits_lock); lock(&q->q_usage_counter(io)#17); lock(&q->limits_lock); lock(&sbi->fat_lock); *** DEADLOCK *** Reported-by: syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> --- drivers/block/loop.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 78a7bb2..5f3ce51 100644 --- a/drivers/block/loop.c 2024-09-16 13:45:20.253220178 +0900 +++ b/drivers/block/loop.c 2024-11-11 21:51:00.910135443 +0900 @@ -770,12 +770,11 @@ static void loop_sysfs_exit(struct loop_ &loop_attribute_group); } -static void loop_config_discard(struct loop_device *lo, - struct queue_limits *lim) +static void loop_get_discard_config(struct loop_device *lo, + u32 *granularity, u32 *max_discard_sectors) { struct file *file = lo->lo_backing_file; struct inode *inode = file->f_mapping->host; - u32 granularity = 0, max_discard_sectors = 0; struct kstatfs sbuf; /* @@ -788,8 +787,9 @@ static void loop_config_discard(struct l if (S_ISBLK(inode->i_mode)) { struct request_queue *backingq = bdev_get_queue(I_BDEV(inode)); - max_discard_sectors = backingq->limits.max_write_zeroes_sectors; - granularity = bdev_discard_granularity(I_BDEV(inode)) ?: + *max_discard_sectors = + backingq->limits.max_write_zeroes_sectors; + *granularity = bdev_discard_granularity(I_BDEV(inode)) ?: queue_physical_block_size(backingq); /* @@ -797,16 +797,9 @@ static void loop_config_discard(struct l * image a.k.a. discard. */ } else if (file->f_op->fallocate && !vfs_statfs(&file->f_path, &sbuf)) { - max_discard_sectors = UINT_MAX >> 9; - granularity = sbuf.f_bsize; + *max_discard_sectors = UINT_MAX >> 9; + *granularity = sbuf.f_bsize; } - - lim->max_hw_discard_sectors = max_discard_sectors; - lim->max_write_zeroes_sectors = max_discard_sectors; - if (max_discard_sectors) - lim->discard_granularity = granularity; - else - lim->discard_granularity = 0; } struct loop_worker { @@ -992,6 +985,7 @@ static int loop_reconfigure_limits(struc struct inode *inode = file->f_mapping->host; struct block_device *backing_bdev = NULL; struct queue_limits lim; + u32 granularity = 0, max_discard_sectors = 0; if (S_ISBLK(inode->i_mode)) backing_bdev = I_BDEV(inode); @@ -1001,6 +995,8 @@ static int loop_reconfigure_limits(struc if (!bsize) bsize = loop_default_blocksize(lo, backing_bdev); + loop_get_discard_config(lo, &granularity, &max_discard_sectors); + lim = queue_limits_start_update(lo->lo_queue); lim.logical_block_size = bsize; lim.physical_block_size = bsize; @@ -1010,7 +1006,12 @@ static int loop_reconfigure_limits(struc lim.features |= BLK_FEAT_WRITE_CACHE; if (backing_bdev && !bdev_nonrot(backing_bdev)) lim.features |= BLK_FEAT_ROTATIONAL; - loop_config_discard(lo, &lim); + lim.max_hw_discard_sectors = max_discard_sectors; + lim.max_write_zeroes_sectors = max_discard_sectors; + if (max_discard_sectors) + lim.discard_granularity = granularity; + else + lim.discard_granularity = 0; return queue_limits_commit_update(lo->lo_queue, &lim); } _ -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] loop: Fix ABBA locking race (Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters) 2024-11-11 13:07 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters OGAWA Hirofumi @ 2024-11-19 7:27 ` OGAWA Hirofumi 2024-11-19 12:10 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters Ming Lei 1 sibling, 0 replies; 7+ messages in thread From: OGAWA Hirofumi @ 2024-11-19 7:27 UTC (permalink / raw) To: Jens Axboe Cc: linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes: ping? > Hi, > > syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: > >> syzbot found the following issue on: >> >> HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 >> git tree: linux-next >> console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 >> kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 >> dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc >> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 > > This patch is to fix the above race. Please check this. > > Thanks > > > From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> > Subject: [PATCH] loop: Fix ABBA locking race > Date: Mon, 11 Nov 2024 21:53:36 +0900 > > Current loop calls vfs_statfs() while holding the q->limits_lock. If > FS takes some locking in vfs_statfs callback, this may lead to ABBA > locking bug (at least, FAT fs has this issue actually). > > So this patch calls vfs_statfs() outside q->limits_locks instead, > because looks like there is no reason to hold q->limits_locks while > getting discard configs. > > Chain exists of: > &sbi->fat_lock --> &q->q_usage_counter(io)#17 --> &q->limits_lock > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(&q->limits_lock); > lock(&q->q_usage_counter(io)#17); > lock(&q->limits_lock); > lock(&sbi->fat_lock); > > *** DEADLOCK *** > > Reported-by: syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc > Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> > --- > drivers/block/loop.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index 78a7bb2..5f3ce51 100644 > --- a/drivers/block/loop.c 2024-09-16 13:45:20.253220178 +0900 > +++ b/drivers/block/loop.c 2024-11-11 21:51:00.910135443 +0900 > @@ -770,12 +770,11 @@ static void loop_sysfs_exit(struct loop_ > &loop_attribute_group); > } > > -static void loop_config_discard(struct loop_device *lo, > - struct queue_limits *lim) > +static void loop_get_discard_config(struct loop_device *lo, > + u32 *granularity, u32 *max_discard_sectors) > { > struct file *file = lo->lo_backing_file; > struct inode *inode = file->f_mapping->host; > - u32 granularity = 0, max_discard_sectors = 0; > struct kstatfs sbuf; > > /* > @@ -788,8 +787,9 @@ static void loop_config_discard(struct l > if (S_ISBLK(inode->i_mode)) { > struct request_queue *backingq = bdev_get_queue(I_BDEV(inode)); > > - max_discard_sectors = backingq->limits.max_write_zeroes_sectors; > - granularity = bdev_discard_granularity(I_BDEV(inode)) ?: > + *max_discard_sectors = > + backingq->limits.max_write_zeroes_sectors; > + *granularity = bdev_discard_granularity(I_BDEV(inode)) ?: > queue_physical_block_size(backingq); > > /* > @@ -797,16 +797,9 @@ static void loop_config_discard(struct l > * image a.k.a. discard. > */ > } else if (file->f_op->fallocate && !vfs_statfs(&file->f_path, &sbuf)) { > - max_discard_sectors = UINT_MAX >> 9; > - granularity = sbuf.f_bsize; > + *max_discard_sectors = UINT_MAX >> 9; > + *granularity = sbuf.f_bsize; > } > - > - lim->max_hw_discard_sectors = max_discard_sectors; > - lim->max_write_zeroes_sectors = max_discard_sectors; > - if (max_discard_sectors) > - lim->discard_granularity = granularity; > - else > - lim->discard_granularity = 0; > } > > struct loop_worker { > @@ -992,6 +985,7 @@ static int loop_reconfigure_limits(struc > struct inode *inode = file->f_mapping->host; > struct block_device *backing_bdev = NULL; > struct queue_limits lim; > + u32 granularity = 0, max_discard_sectors = 0; > > if (S_ISBLK(inode->i_mode)) > backing_bdev = I_BDEV(inode); > @@ -1001,6 +995,8 @@ static int loop_reconfigure_limits(struc > if (!bsize) > bsize = loop_default_blocksize(lo, backing_bdev); > > + loop_get_discard_config(lo, &granularity, &max_discard_sectors); > + > lim = queue_limits_start_update(lo->lo_queue); > lim.logical_block_size = bsize; > lim.physical_block_size = bsize; > @@ -1010,7 +1006,12 @@ static int loop_reconfigure_limits(struc > lim.features |= BLK_FEAT_WRITE_CACHE; > if (backing_bdev && !bdev_nonrot(backing_bdev)) > lim.features |= BLK_FEAT_ROTATIONAL; > - loop_config_discard(lo, &lim); > + lim.max_hw_discard_sectors = max_discard_sectors; > + lim.max_write_zeroes_sectors = max_discard_sectors; > + if (max_discard_sectors) > + lim.discard_granularity = granularity; > + else > + lim.discard_granularity = 0; > return queue_limits_commit_update(lo->lo_queue, &lim); > } > > _ -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters 2024-11-11 13:07 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters OGAWA Hirofumi 2024-11-19 7:27 ` [PATCH] loop: Fix ABBA locking race (Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters) OGAWA Hirofumi @ 2024-11-19 12:10 ` Ming Lei 2024-11-19 14:18 ` Jens Axboe 1 sibling, 1 reply; 7+ messages in thread From: Ming Lei @ 2024-11-19 12:10 UTC (permalink / raw) To: OGAWA Hirofumi Cc: Jens Axboe, linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs On Tue, Nov 12, 2024 at 12:44 AM OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote: > > Hi, > > syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: > > > syzbot found the following issue on: > > > > HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 > > git tree: linux-next > > console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 > > dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc > > compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 > > This patch is to fix the above race. Please check this. > > Thanks > > > From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> > Subject: [PATCH] loop: Fix ABBA locking race > Date: Mon, 11 Nov 2024 21:53:36 +0900 > > Current loop calls vfs_statfs() while holding the q->limits_lock. If > FS takes some locking in vfs_statfs callback, this may lead to ABBA > locking bug (at least, FAT fs has this issue actually). > > So this patch calls vfs_statfs() outside q->limits_locks instead, > because looks like there is no reason to hold q->limits_locks while > getting discard configs. > > Chain exists of: > &sbi->fat_lock --> &q->q_usage_counter(io)#17 --> &q->limits_lock > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(&q->limits_lock); > lock(&q->q_usage_counter(io)#17); > lock(&q->limits_lock); > lock(&sbi->fat_lock); > > *** DEADLOCK *** > > Reported-by: syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc > Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> > --- > drivers/block/loop.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index 78a7bb2..5f3ce51 100644 > --- a/drivers/block/loop.c 2024-09-16 13:45:20.253220178 +0900 > +++ b/drivers/block/loop.c 2024-11-11 21:51:00.910135443 +0900 > @@ -770,12 +770,11 @@ static void loop_sysfs_exit(struct loop_ > &loop_attribute_group); > } > > -static void loop_config_discard(struct loop_device *lo, > - struct queue_limits *lim) > +static void loop_get_discard_config(struct loop_device *lo, > + u32 *granularity, u32 *max_discard_sectors) > { > struct file *file = lo->lo_backing_file; > struct inode *inode = file->f_mapping->host; > - u32 granularity = 0, max_discard_sectors = 0; > struct kstatfs sbuf; > > /* > @@ -788,8 +787,9 @@ static void loop_config_discard(struct l > if (S_ISBLK(inode->i_mode)) { > struct request_queue *backingq = bdev_get_queue(I_BDEV(inode)); > > - max_discard_sectors = backingq->limits.max_write_zeroes_sectors; > - granularity = bdev_discard_granularity(I_BDEV(inode)) ?: > + *max_discard_sectors = > + backingq->limits.max_write_zeroes_sectors; > + *granularity = bdev_discard_granularity(I_BDEV(inode)) ?: > queue_physical_block_size(backingq); > > /* > @@ -797,16 +797,9 @@ static void loop_config_discard(struct l > * image a.k.a. discard. > */ > } else if (file->f_op->fallocate && !vfs_statfs(&file->f_path, &sbuf)) { > - max_discard_sectors = UINT_MAX >> 9; > - granularity = sbuf.f_bsize; > + *max_discard_sectors = UINT_MAX >> 9; > + *granularity = sbuf.f_bsize; > } > - > - lim->max_hw_discard_sectors = max_discard_sectors; > - lim->max_write_zeroes_sectors = max_discard_sectors; > - if (max_discard_sectors) > - lim->discard_granularity = granularity; > - else > - lim->discard_granularity = 0; > } > > struct loop_worker { > @@ -992,6 +985,7 @@ static int loop_reconfigure_limits(struc > struct inode *inode = file->f_mapping->host; > struct block_device *backing_bdev = NULL; > struct queue_limits lim; > + u32 granularity = 0, max_discard_sectors = 0; > > if (S_ISBLK(inode->i_mode)) > backing_bdev = I_BDEV(inode); > @@ -1001,6 +995,8 @@ static int loop_reconfigure_limits(struc > if (!bsize) > bsize = loop_default_blocksize(lo, backing_bdev); > > + loop_get_discard_config(lo, &granularity, &max_discard_sectors); > + > lim = queue_limits_start_update(lo->lo_queue); > lim.logical_block_size = bsize; > lim.physical_block_size = bsize; > @@ -1010,7 +1006,12 @@ static int loop_reconfigure_limits(struc > lim.features |= BLK_FEAT_WRITE_CACHE; > if (backing_bdev && !bdev_nonrot(backing_bdev)) > lim.features |= BLK_FEAT_ROTATIONAL; > - loop_config_discard(lo, &lim); > + lim.max_hw_discard_sectors = max_discard_sectors; > + lim.max_write_zeroes_sectors = max_discard_sectors; > + if (max_discard_sectors) > + lim.discard_granularity = granularity; > + else > + lim.discard_granularity = 0; > return queue_limits_commit_update(lo->lo_queue, &lim); > } Looks fine, Reviewed-by: Ming Lei <ming.lei@redhat.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters 2024-11-19 12:10 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters Ming Lei @ 2024-11-19 14:18 ` Jens Axboe 2024-11-19 14:46 ` OGAWA Hirofumi 0 siblings, 1 reply; 7+ messages in thread From: Jens Axboe @ 2024-11-19 14:18 UTC (permalink / raw) To: Ming Lei, OGAWA Hirofumi Cc: linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs On 11/19/24 5:10 AM, Ming Lei wrote: > On Tue, Nov 12, 2024 at 12:44 AM OGAWA Hirofumi > <hirofumi@mail.parknet.co.jp> wrote: >> >> Hi, >> >> syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: >> >>> syzbot found the following issue on: >>> >>> HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 >>> git tree: linux-next >>> console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 >>> kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 >>> dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc >>> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 >> >> This patch is to fix the above race. Please check this. >> >> Thanks >> >> >> From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> >> Subject: [PATCH] loop: Fix ABBA locking race >> Date: Mon, 11 Nov 2024 21:53:36 +0900 >> >> Current loop calls vfs_statfs() while holding the q->limits_lock. If >> FS takes some locking in vfs_statfs callback, this may lead to ABBA >> locking bug (at least, FAT fs has this issue actually). >> >> So this patch calls vfs_statfs() outside q->limits_locks instead, >> because looks like there is no reason to hold q->limits_locks while >> getting discard configs. >> >> Chain exists of: >> &sbi->fat_lock --> &q->q_usage_counter(io)#17 --> &q->limits_lock >> >> Possible unsafe locking scenario: >> >> CPU0 CPU1 >> ---- ---- >> lock(&q->limits_lock); >> lock(&q->q_usage_counter(io)#17); >> lock(&q->limits_lock); >> lock(&sbi->fat_lock); >> >> *** DEADLOCK *** >> >> Reported-by: syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc >> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> >> --- >> drivers/block/loop.c | 31 ++++++++++++++++--------------- >> 1 file changed, 16 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/block/loop.c b/drivers/block/loop.c >> index 78a7bb2..5f3ce51 100644 >> --- a/drivers/block/loop.c 2024-09-16 13:45:20.253220178 +0900 >> +++ b/drivers/block/loop.c 2024-11-11 21:51:00.910135443 +0900 >> @@ -770,12 +770,11 @@ static void loop_sysfs_exit(struct loop_ >> &loop_attribute_group); >> } >> >> -static void loop_config_discard(struct loop_device *lo, >> - struct queue_limits *lim) >> +static void loop_get_discard_config(struct loop_device *lo, >> + u32 *granularity, u32 *max_discard_sectors) >> { >> struct file *file = lo->lo_backing_file; >> struct inode *inode = file->f_mapping->host; >> - u32 granularity = 0, max_discard_sectors = 0; >> struct kstatfs sbuf; >> >> /* >> @@ -788,8 +787,9 @@ static void loop_config_discard(struct l >> if (S_ISBLK(inode->i_mode)) { >> struct request_queue *backingq = bdev_get_queue(I_BDEV(inode)); >> >> - max_discard_sectors = backingq->limits.max_write_zeroes_sectors; >> - granularity = bdev_discard_granularity(I_BDEV(inode)) ?: >> + *max_discard_sectors = >> + backingq->limits.max_write_zeroes_sectors; >> + *granularity = bdev_discard_granularity(I_BDEV(inode)) ?: >> queue_physical_block_size(backingq); >> >> /* >> @@ -797,16 +797,9 @@ static void loop_config_discard(struct l >> * image a.k.a. discard. >> */ >> } else if (file->f_op->fallocate && !vfs_statfs(&file->f_path, &sbuf)) { >> - max_discard_sectors = UINT_MAX >> 9; >> - granularity = sbuf.f_bsize; >> + *max_discard_sectors = UINT_MAX >> 9; >> + *granularity = sbuf.f_bsize; >> } >> - >> - lim->max_hw_discard_sectors = max_discard_sectors; >> - lim->max_write_zeroes_sectors = max_discard_sectors; >> - if (max_discard_sectors) >> - lim->discard_granularity = granularity; >> - else >> - lim->discard_granularity = 0; >> } >> >> struct loop_worker { >> @@ -992,6 +985,7 @@ static int loop_reconfigure_limits(struc >> struct inode *inode = file->f_mapping->host; >> struct block_device *backing_bdev = NULL; >> struct queue_limits lim; >> + u32 granularity = 0, max_discard_sectors = 0; >> >> if (S_ISBLK(inode->i_mode)) >> backing_bdev = I_BDEV(inode); >> @@ -1001,6 +995,8 @@ static int loop_reconfigure_limits(struc >> if (!bsize) >> bsize = loop_default_blocksize(lo, backing_bdev); >> >> + loop_get_discard_config(lo, &granularity, &max_discard_sectors); >> + >> lim = queue_limits_start_update(lo->lo_queue); >> lim.logical_block_size = bsize; >> lim.physical_block_size = bsize; >> @@ -1010,7 +1006,12 @@ static int loop_reconfigure_limits(struc >> lim.features |= BLK_FEAT_WRITE_CACHE; >> if (backing_bdev && !bdev_nonrot(backing_bdev)) >> lim.features |= BLK_FEAT_ROTATIONAL; >> - loop_config_discard(lo, &lim); >> + lim.max_hw_discard_sectors = max_discard_sectors; >> + lim.max_write_zeroes_sectors = max_discard_sectors; >> + if (max_discard_sectors) >> + lim.discard_granularity = granularity; >> + else >> + lim.discard_granularity = 0; >> return queue_limits_commit_update(lo->lo_queue, &lim); >> } > > Looks fine, > > Reviewed-by: Ming Lei <ming.lei@redhat.com> The patch doesn't apply to the for-6.13/block tree, Ogawa can you send an updated one please? -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters 2024-11-19 14:18 ` Jens Axboe @ 2024-11-19 14:46 ` OGAWA Hirofumi 2024-11-19 14:55 ` Jens Axboe 0 siblings, 1 reply; 7+ messages in thread From: OGAWA Hirofumi @ 2024-11-19 14:46 UTC (permalink / raw) To: Jens Axboe Cc: Ming Lei, linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs Jens Axboe <axboe@kernel.dk> writes: > On 11/19/24 5:10 AM, Ming Lei wrote: >> On Tue, Nov 12, 2024 at 12:44 AM OGAWA Hirofumi >> <hirofumi@mail.parknet.co.jp> wrote: >>> >>> Hi, >>> >>> syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: >>> >>>> syzbot found the following issue on: >>>> >>>> HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 >>>> git tree: linux-next >>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 >>>> kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 >>>> dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc >>>> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 [...] >> >> Looks fine, >> >> Reviewed-by: Ming Lei <ming.lei@redhat.com> > > The patch doesn't apply to the for-6.13/block tree, Ogawa can you send > an updated one please? Updated the patch for linux-block:for-6.13/block. Please apply. Thanks. From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Subject: [PATCH] loop: Fix ABBA locking race Date: Tue, 19 Nov 2024 23:42:23 +0900 Current loop calls vfs_statfs() while holding the q->limits_lock. If FS takes some locking in vfs_statfs callback, this may lead to ABBA locking bug (at least, FAT fs has this issue actually). So this patch calls vfs_statfs() outside q->limits_locks instead, because looks like no reason to hold q->limits_locks while getting discord configs. Chain exists of: &sbi->fat_lock --> &q->q_usage_counter(io)#17 --> &q->limits_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&q->limits_lock); lock(&q->q_usage_counter(io)#17); lock(&q->limits_lock); lock(&sbi->fat_lock); *** DEADLOCK *** Reported-by: syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc Reviewed-by: Ming Lei <ming.lei@redhat.com> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> --- drivers/block/loop.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/block/loop.c b/drivers/block/loop.c index fe9bb4f..8f6761c 100644 --- a/drivers/block/loop.c 2024-11-19 23:37:54.760751014 +0900 +++ b/drivers/block/loop.c 2024-11-19 23:38:55.645461107 +0900 @@ -770,12 +770,11 @@ static void loop_sysfs_exit(struct loop_ &loop_attribute_group); } -static void loop_config_discard(struct loop_device *lo, - struct queue_limits *lim) +static void loop_get_discard_config(struct loop_device *lo, + u32 *granularity, u32 *max_discard_sectors) { struct file *file = lo->lo_backing_file; struct inode *inode = file->f_mapping->host; - u32 granularity = 0, max_discard_sectors = 0; struct kstatfs sbuf; /* @@ -788,24 +787,17 @@ static void loop_config_discard(struct l if (S_ISBLK(inode->i_mode)) { struct block_device *bdev = I_BDEV(inode); - max_discard_sectors = bdev_write_zeroes_sectors(bdev); - granularity = bdev_discard_granularity(bdev); + *max_discard_sectors = bdev_write_zeroes_sectors(bdev); + *granularity = bdev_discard_granularity(bdev); /* * We use punch hole to reclaim the free space used by the * image a.k.a. discard. */ } else if (file->f_op->fallocate && !vfs_statfs(&file->f_path, &sbuf)) { - max_discard_sectors = UINT_MAX >> 9; - granularity = sbuf.f_bsize; + *max_discard_sectors = UINT_MAX >> 9; + *granularity = sbuf.f_bsize; } - - lim->max_hw_discard_sectors = max_discard_sectors; - lim->max_write_zeroes_sectors = max_discard_sectors; - if (max_discard_sectors) - lim->discard_granularity = granularity; - else - lim->discard_granularity = 0; } struct loop_worker { @@ -991,6 +983,7 @@ static int loop_reconfigure_limits(struc struct inode *inode = file->f_mapping->host; struct block_device *backing_bdev = NULL; struct queue_limits lim; + u32 granularity = 0, max_discard_sectors = 0; if (S_ISBLK(inode->i_mode)) backing_bdev = I_BDEV(inode); @@ -1000,6 +993,8 @@ static int loop_reconfigure_limits(struc if (!bsize) bsize = loop_default_blocksize(lo, backing_bdev); + loop_get_discard_config(lo, &granularity, &max_discard_sectors); + lim = queue_limits_start_update(lo->lo_queue); lim.logical_block_size = bsize; lim.physical_block_size = bsize; @@ -1009,7 +1004,12 @@ static int loop_reconfigure_limits(struc lim.features |= BLK_FEAT_WRITE_CACHE; if (backing_bdev && !bdev_nonrot(backing_bdev)) lim.features |= BLK_FEAT_ROTATIONAL; - loop_config_discard(lo, &lim); + lim.max_hw_discard_sectors = max_discard_sectors; + lim.max_write_zeroes_sectors = max_discard_sectors; + if (max_discard_sectors) + lim.discard_granularity = granularity; + else + lim.discard_granularity = 0; return queue_limits_commit_update(lo->lo_queue, &lim); } _ -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters 2024-11-19 14:46 ` OGAWA Hirofumi @ 2024-11-19 14:55 ` Jens Axboe 2024-11-19 15:12 ` OGAWA Hirofumi 0 siblings, 1 reply; 7+ messages in thread From: Jens Axboe @ 2024-11-19 14:55 UTC (permalink / raw) To: OGAWA Hirofumi Cc: Ming Lei, linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs On 11/19/24 7:46 AM, OGAWA Hirofumi wrote: > Jens Axboe <axboe@kernel.dk> writes: > >> On 11/19/24 5:10 AM, Ming Lei wrote: >>> On Tue, Nov 12, 2024 at 12:44?AM OGAWA Hirofumi >>> <hirofumi@mail.parknet.co.jp> wrote: >>>> >>>> Hi, >>>> >>>> syzbot <syzbot+a5d8c609c02f508672cc@syzkaller.appspotmail.com> writes: >>>> >>>>> syzbot found the following issue on: >>>>> >>>>> HEAD commit: 929beafbe7ac Add linux-next specific files for 20241108 >>>>> git tree: linux-next >>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1621bd87980000 >>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=75175323f2078363 >>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=a5d8c609c02f508672cc >>>>> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 > > [...] > >>> >>> Looks fine, >>> >>> Reviewed-by: Ming Lei <ming.lei@redhat.com> >> >> The patch doesn't apply to the for-6.13/block tree, Ogawa can you send >> an updated one please? > > Updated the patch for linux-block:for-6.13/block. Please apply. Applied, thanks. FWIW, your outgoing mailer is mangling patches. I fixed it up manually, but probably something you want to get sorted. Download the raw one from lore and you can see what I mean. -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters 2024-11-19 14:55 ` Jens Axboe @ 2024-11-19 15:12 ` OGAWA Hirofumi 0 siblings, 0 replies; 7+ messages in thread From: OGAWA Hirofumi @ 2024-11-19 15:12 UTC (permalink / raw) To: Jens Axboe Cc: Ming Lei, linux-block, syzbot, linkinjeon, linux-fsdevel, linux-kernel, sj1557.seo, syzkaller-bugs Jens Axboe <axboe@kernel.dk> writes: > FWIW, your outgoing mailer is mangling patches. I fixed it up manually, > but probably something you want to get sorted. Download the raw one from > lore and you can see what I mean. Looks like at Ming Lei's reply, unicode "NARROW NO-BREAK SPACE" was included in ">>>> On Tue, Nov 12, 2024 at 12:44?AM OGAWA Hirofumi" line? So my mailer may be encoded as utf-8, not raw. I'll take more care next time if possible. However, this mistake (utf-8 whitespace) may hard to prevent without machinery check somehow. Thanks. -- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-11-19 15:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <67313d9e.050a0220.138bd5.0054.GAE@google.com>
2024-11-11 13:07 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters OGAWA Hirofumi
2024-11-19 7:27 ` [PATCH] loop: Fix ABBA locking race (Re: [syzbot] [exfat?] possible deadlock in fat_count_free_clusters) OGAWA Hirofumi
2024-11-19 12:10 ` [syzbot] [exfat?] possible deadlock in fat_count_free_clusters Ming Lei
2024-11-19 14:18 ` Jens Axboe
2024-11-19 14:46 ` OGAWA Hirofumi
2024-11-19 14:55 ` Jens Axboe
2024-11-19 15:12 ` OGAWA Hirofumi
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).