Message ID | 0a3faca1c52f7fff0ac35566c6453f81f57a3d16.camel@rong.moe (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | scrub: slab-use-after-free (ae76d8e3e) | expand |
On 2024/1/14 04:10, Rongrong wrote: > Hi all, > > I recently scrubbed my fs and surprisingly saw kernel oops. For context > and more information about the fs, please refer to > https://lore.kernel.org/linux-btrfs/8bd12a1ee60172f53ee0c27374f41c3ec9976be8.camel@rong.moe > > Usually the kernel oops when the scrub progress is 1~30%. Then it will > oops again, making the btrfs process die. After that, any attempt to > unmount the fs hangs forever, so do reboot and shutdown. > > It seems always reproducible if any of the below conditions is met: > - Intel CPU (i7-7567U/i7-13700H), intel_iommu=off > - x86_64 KVM (host CPU: Intel/AMD), virtio-blk > - RK3399 > - Mounted via a loop device > > Not reproducible on: > - Intel CPU (i7-7567U/i7-13700H), intel_iommu=on > - AMD Ryzen CPU (PRO 4750U), amd_iommu=on/off > > Starting here, all experiments were done in a KVM: > - Host: Intel i7-7567U > - Vdisk: raw dump of my fs, virtio-blk (IO scheduler: none) > - btrfs-progs: 6.6.3 > > The first reproducible stable kernel is 6.5.4. > > Bisection between 6.5.3 and 6.5.4 indicates that the buggy commit is > ce585c9b1cd700324fecedae130a9c35261fec30, which is a backport of > ae76d8e3e1351aa1ba09cc68dab6866d356f2e17 (first appeared in v6.6-rc1). > > KASAN detected slab-use-after-free in __blk_rq_map_sg while scrubbing > on v6.7, meanwhile an assertion was failed at block/blk-merge.c:584. > See also the attached dmesg. > > With the below patch applied to v6.7, slab-use-after-free and oops > disappeared, making scrub able to finish. Thanks for the report. Although I believe the root cause is the same for the uncorrectable errors. And IMHO, even with the patch, as long as you enable KASAN, KASAN would still warn about the use-after-free. The root cause is that, the new scrub code expects the whole 64K read to be mapped in one go, thus no bio split from btrfs bio layer. But for the converted fs, the end of a data chunk is not ensured to end at the 64K boundary, thus it can lead to unexpected bio split. With the extra bio split, the scrub_read_endio() can be called twice, causing the same bio to be freed and all kind of weird use-after-free. Sorry I didn't notice it early enough, as it needs the full chunk tree dump to verify. So all of these would be handled in the original thread. Thanks, Qu > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index b877203f1dc5..3fecd7eee0bf 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -1698,14 +1698,11 @@ static void submit_initial_group_read(struct scrub_ctx *sctx, > unsigned int first_slot, > unsigned int nr_stripes) > { > - struct blk_plug plug; > - > ASSERT(first_slot < SCRUB_TOTAL_STRIPES); > ASSERT(first_slot + nr_stripes <= SCRUB_TOTAL_STRIPES); > > scrub_throttle_dev_io(sctx, sctx->stripes[0].dev, > btrfs_stripe_nr_to_offset(nr_stripes)); > - blk_start_plug(&plug); > for (int i = 0; i < nr_stripes; i++) { > struct scrub_stripe *stripe = &sctx->stripes[first_slot + i]; > > @@ -1713,7 +1710,6 @@ static void submit_initial_group_read(struct scrub_ctx *sctx, > ASSERT(test_bit(SCRUB_STRIPE_FLAG_INITIALIZED, &stripe->state)); > scrub_submit_initial_read(sctx, stripe); > } > - blk_finish_plug(&plug); > } > > static int flush_scrub_stripes(struct scrub_ctx *sctx) > > Thanks, > Rongrong
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index b877203f1dc5..3fecd7eee0bf 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -1698,14 +1698,11 @@ static void submit_initial_group_read(struct scrub_ctx *sctx, unsigned int first_slot, unsigned int nr_stripes) { - struct blk_plug plug; - ASSERT(first_slot < SCRUB_TOTAL_STRIPES); ASSERT(first_slot + nr_stripes <= SCRUB_TOTAL_STRIPES); scrub_throttle_dev_io(sctx, sctx->stripes[0].dev, btrfs_stripe_nr_to_offset(nr_stripes)); - blk_start_plug(&plug); for (int i = 0; i < nr_stripes; i++) { struct scrub_stripe *stripe = &sctx->stripes[first_slot + i]; @@ -1713,7 +1710,6 @@ static void submit_initial_group_read(struct scrub_ctx *sctx, ASSERT(test_bit(SCRUB_STRIPE_FLAG_INITIALIZED, &stripe->state)); scrub_submit_initial_read(sctx, stripe); } - blk_finish_plug(&plug); } static int flush_scrub_stripes(struct scrub_ctx *sctx)