On Fri, 23 Oct 2026, Mike Snitzer wrote:
> On Thu, Oct 22, 2026 at 05:25:02PM +0000, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> > - [Critical] block: blk_set_stacking_limits() corrupts data in dm-ebs and
> > dm-writecache
> > --
> >
> > commit da5c2e4cde4a12a4f8b254dc27480cf9078b36d2
> > Author: Mike Snitzer <[email protected]>
> >
> > block: let a stacking device inherit its members' DMA alignment
> >
> > This commit removes the default sector-size dma_alignment initialization in
> > blk_set_stacking_limits() so that stacking devices inherit the strictest
> > dma_alignment of their underlying member devices. When no member devices are
> > stacked, blk_validate_limits() still applies the default sector alignment.
> >
> > > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > > index e469baa1f08b7..36fcbecdf793b 100644
> > > --- a/block/blk-settings.c
> > > +++ b/block/blk-settings.c
> > > @@ -41,7 +41,14 @@ void blk_set_stacking_limits(struct queue_limits *lim)
> > > lim->physical_block_size = SECTOR_SIZE;
> > > lim->io_min = SECTOR_SIZE;
> > > lim->discard_granularity = SECTOR_SIZE;
> > > - lim->dma_alignment = SECTOR_SIZE - 1;
> > > + /*
> > > + * No dma_alignment of its own: blk_stack_limits() keeps the strictest
> > > + * of the devices stacked under it, and blk_validate_limits() gives the
> > > + * sector-size default to a device left with none. A driver that looks
> > > + * at the data and needs it laid out by logical block says so in its
> > > + * own limits, as dm-crypt, dm-integrity, dm-verity and dm-log-writes
> > > + * do.
> > > + */
> >
> > [Severity: Critical]
> > Could leaving lim->dma_alignment at 0 in blk_set_stacking_limits() cause
> > data
> > corruption regressions in targets like dm-ebs and dm-writecache that inspect
> > bio data but do not set limits->dma_alignment in their io_hints callbacks?
> >
> > When stacked over a device with sub-sector dma_alignment (such as an nvme
> > device with dma_alignment = 3), direct I/O using a 4-byte-aligned user
> > buffer
> > that crosses a page boundary or uses sub-sector iovecs can produce bio_vec
> > segments whose lengths are not multiples of 512 bytes (for example,
> > bv_len = 4092 followed by bv_len = 4).
>
> The code comment in the patch calls out the fact that
> blk_validate_limits() will backfill dma_alignment if none is set (by
> usual limits stacking or explicitly by DM targets).
>
> But sure, there is potential for dragons if stacked block devices
> like DM implicitly relied on block-core's coarse-grained default
> dma_alignment.
>
> Mikulas, would you be open to auditing the DM targets relative to
> dma_alignment stacking after you're done with reviewing Keith's
> related dm-crypt patchset?
Yes, I think that this is the only way forward - and this audit should be
done before accepting the dm-crypt patches.
DM cannot rely on stacking limits, because the DM stack may be
reconfigured anytime - so there is a race condition that a dm-device may
advertise dma alignment 3, someone may construct a bio that uses 4-byte
aligned vectors, the stack may be reconfigured with a completely different
target and the bio may be sent to this different target and cause trouble.
So, we need to be prepared to handle any alignment at device mapper entry.
I think we should add a new flag "accepts_unaligned_bio_vector" to struct
dm_target. If the flag is not present, then the dm-core would check the
alignment and reject bios with BLK_STS_INVAL in the dm core.
Then, we can gradually enable the flag for more and more targets as they
are checked.
Mikulas
> Sorry for the extra work... but its worthwhile for DM to be more
> precise with how it stacks dma_alignment
>
> Thanks,
> Mike
