Re: [PATCH 2/3] block: cap atomic write size by PI buffer size constraints
From: John Garry <hidden>
Date: 2026-09-10 12:52:13
On 9/10/26 09:45, John Garry wrote:
quoted
quoted
The expectation is that the driver sets atomic_write_hw_max, atomic_write_hw_unit_min, and atomic_write_hw_unit_max individually. We don't assume that atomic_write_hw_unit_max == rounddown_pow_of_two(atomic_write_hw_max). The is because of SCSI and it's granularity and alignment atomic limits 🙁 Anyway, maybe this is a better change:diff --git a/block/blk-settings.c b/block/blk-settings.c index 8274631290db..b88744d42719 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c@@ -233,11 +233,15 @@ static void blk_atomic_writes_update_limits(structqueue_limits *lim) unsigned int unit_limit = min(lim->max_hw_sectors << SECTOR_SHIFT, blk_queue_max_guaranteed_bio(lim)); + unit_limit = min_not_zero(unit_limit, max_integrity_io_size(lim)); unit_limit = rounddown_pow_of_two(unit_limit); lim->atomic_write_max_sectors = min(lim->atomic_write_hw_max >> SECTOR_SHIFT, lim->max_hw_sectors); + lim->atomic_write_max_sectors = + min_not_zero(lim->atomic_write_max_sectors, + max_integrity_io_size(lim));This needs to be conditional on a non-zero lim->integrity.metadata_size. Otherwise this looks sane.ok, I can add that. I'd like to test this, I suppose scsi_debug is all I have to sanity test this...
It's a bit tricky to test this, as max_integrity_io_size() >> atomic_write_unit_max normally and so I could not exceed that buffer with an atomic write. However with these changes:
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h@@ -1806,7 +1806,7 @@ static inline int bio_split_rw_at(struct bio *bio, /* * Maximum contiguous integrity buffer allocation. */ -#define BLK_INTEGRITY_MAX_SIZE SZ_2M +#define BLK_INTEGRITY_MAX_SIZE SZ_4K --- a/block/bio-integrity.c +++ b/block/bio-integrity.c
@@ -72,7 +72,7 @@ void bio_integrity_alloc_buf(struct bio *bio, gfp_t gfp, bool zero_buffer)
unsigned int len = bio_integrity_bytes(bi, bio_sectors(bio));
void *buf;
- buf = kmalloc(len, gfp | __GFP_NOWARN | (zero_buffer ?
__GFP_ZERO : 0));
+ buf = NULL;//kmalloc(len, gfp | __GFP_NOWARN | (zero_buffer ?
__GFP_ZERO : 0));
if (unlikely(!buf)) {
struct page *page;
I can trigger a KASAN use-after-free report:
# cat /sys/block/sda/queue/atomic_write_unit_max_bytes
524288
# xfs_io -d -C "pwrite -b 64k -V 1 -A -D 0 64k" /dev/sda
wrote 65536/65536 bytes at offset 0
64 KiB, 1 ops; 0.0512 sec (1.220 MiB/sec and 19.5164 ops/sec)
# xfs_io -d -C "pwrite -b 512k -V 1 -A -D 0 512k" /dev/sda
[ 39.779692]
==================================================================
[ 39.779698] BUG: KASAN: use-after-free in
blk_integrity_iterate+0x18d5/0x1bb0
[ 39.779724] Write of size 2 at addr ffff888118160000 by task xfs_io/244
[ 39.779726]
[ 39.779729] CPU: 3 UID: 0 PID: 244 Comm: xfs_io Not tainted
7.3.0-rc2-00042-gb7b6d996bb84-dirty #1238 PREEMPT(lazy)
[ 39.779732] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.16.3-debian-1.16.3-2 04/01/2014
[ 39.779734] Call Trace:
[ 39.779736] <TASK>
[ 39.779737] dump_stack_lvl+0x68/0xa0
[ 39.779743] print_report+0x10d/0x5d0
[ 39.779747] ? __virt_addr_valid+0x21e/0x3f0
[ 39.779752] ? blk_integrity_iterate+0x18d5/0x1bb0
[ 39.779754] kasan_report+0x96/0xd0
[ 39.779757] ? blk_integrity_iterate+0x18d5/0x1bb0
[ 39.779760] blk_integrity_iterate+0x18d5/0x1bb0
[ 39.779764] ? alloc_pages_noprof+0x25/0x70
[ 39.779766] ? mempool_alloc_noprof+0x129/0x1f0
[ 39.779770] ? __pfx_blk_integrity_iterate+0x10/0x10
[ 39.779774] ? bio_integrity_alloc_buf+0x191/0x4a0
[ 39.779776] ? bio_integrity_init+0x25/0xf0
[ 39.779779] blk_mq_submit_bio+0xc21/0x2600
[ 39.779784] ? __pfx_blk_mq_submit_bio+0x10/0x10
[ 39.779786] ? kasan_save_stack+0x34/0x50
[ 39.779788] ? kasan_save_stack+0x24/0x50
[ 39.779801] ? __pfx_iov_iter_extract_pages+0x10/0x10
[ 39.779805] ? __blkdev_direct_IO_simple+0xb5/0x910
[ 39.779809] ? blkdev_write_iter+0x707/0xb80
[ 39.779811] ? do_iter_readv_writev+0x3e4/0x7b0
[ 39.779814] ? __pfx_blk_cgroup_bio_start+0x10/0x10
[ 39.779818] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 39.779822] submit_bio_noacct_nocheck+0x474/0xab0
[ 39.779826] ? __pfx_submit_bio_noacct_nocheck+0x10/0x10
[ 39.779829] ? submit_bio_noacct+0x5bb/0x1330
[ 39.779832] bio_await+0x160/0x190
[ 39.779835] ? __pfx_bio_await+0x10/0x10
[ 39.779836] ? find_held_lock+0x2b/0x80
[ 39.779842] submit_bio_wait+0x11/0x50
[ 39.779844] __blkdev_direct_IO_simple+0x404/0x910
[ 39.779847] ? __pfx___blkdev_direct_IO_simple+0x10/0x10
[ 39.779850] ? __pfx_bio_wait_end_io+0x10/0x10
[ 39.779853] ? inode_set_ctime_current+0x7e/0x610
[ 39.779856] ? inode_set_ctime_to_ts+0xf8/0x330
[ 39.779858] ? __pfx_inode_set_ctime_to_ts+0x10/0x10
[ 39.779861] ? ktime_get_coarse_real_ts64_mg+0x196/0x230
[ 39.779864] ? inode_set_ctime_current+0x7e/0x610
[ 39.779867] blkdev_direct_IO+0xa73/0x1e10
[ 39.779870] ? inode_maybe_inc_iversion+0xe3/0x1a0
[ 39.779874] ? __pfx_inode_maybe_inc_iversion+0x10/0x10
[ 39.779876] ? lock_acquire+0x185/0x2e0
[ 39.779880] ? __pfx_blkdev_direct_IO+0x10/0x10
[ 39.779882] ? __mark_inode_dirty+0x95d/0x10c0
[ 39.779886] ? generic_update_time+0x8c/0xf0
[ 39.779888] blkdev_write_iter+0x707/0xb80
[ 39.779892] do_iter_readv_writev+0x3e4/0x7b0
[ 39.779895] ? __pfx_do_iter_readv_writev+0x10/0x10
[ 39.779897] ? folio_add_lru_vma+0x15b/0x1d0
[ 39.779901] ? selinux_file_permission+0x361/0x4d0
[ 39.779905] vfs_writev+0x269/0xb40
[ 39.779907] ? lock_release+0xc9/0x290
[ 39.779911] ? __pfx_vfs_writev+0x10/0x10
[ 39.779913] ? lock_vma_under_rcu+0x2e1/0x960
[ 39.779917] ? lock_release+0x124/0x290
[ 39.779920] ? lock_vma_under_rcu+0x2eb/0x960
[ 39.779925] ? do_pwritev+0x144/0x220
[ 39.779927] do_pwritev+0x144/0x220
[ 39.779929] ? __pfx_do_pwritev+0x10/0x10
[ 39.779931] ? irqentry_exit+0xf8/0x860
[ 39.779934] ? trace_hardirqs_on_prepare+0x11d/0x150
[ 39.779937] ? irqentry_exit+0x112/0x860
[ 39.779940] do_syscall_64+0x11a/0x610
[ 39.779943] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 39.779946] RIP: 0033:0x7f391f1c9727
[
...
Now I'll fix that patch as suggested and retest.