On Tue, Apr 28, 2026 at 06:43:08PM +0800, Linlin Zhang wrote: > Correct the response to Benjamin's comments. > > On 4/27/2026 8:20 PM, Linlin Zhang wrote: > > > > > > On 4/27/2026 9:19 AM, Benjamin Marzinski wrote: > >> On Fri, Apr 10, 2026 at 06:40:30AM -0700, Linlin Zhang wrote: > >>> From: Eric Biggers <[email protected]> > >>> + > >>> +static int inlinecrypt_map(struct dm_target *ti, struct bio *bio) > >>> +{ > >>> + const struct inlinecrypt_ctx *ctx = ti->private; > >>> + sector_t sector_in_target; > >>> + u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE] = {}; > >>> + > >>> + bio_set_dev(bio, ctx->dev->bdev); > >>> + > >>> + /* > >>> + * If the bio is a device-level request which doesn't target a specific > >>> + * sector, there's nothing more to do. > >>> + */ > >>> + if (bio_sectors(bio) == 0) > >>> + return DM_MAPIO_REMAPPED; > >>> + > >>> + /* > >>> + * The bio should never have an encryption context already, since > >>> + * dm-inlinecrypt doesn't pass through any inline encryption > >>> + * capabilities to the layer above it. > >>> + */ > >>> + if (WARN_ON_ONCE(bio_has_crypt_ctx(bio))) > >>> + return DM_MAPIO_KILL; > >>> + > >>> + /* Map the bio's sector to the underlying device. (512-byte sectors) */ > >>> + sector_in_target = dm_target_offset(ti, bio->bi_iter.bi_sector); > >>> + bio->bi_iter.bi_sector = ctx->start + sector_in_target; > >>> + /* > >>> + * If the bio doesn't have any data (e.g. if it's a DISCARD request), > >>> + * there's nothing more to do. > >>> + */ > >>> + if (!bio_has_data(bio)) > >>> + return DM_MAPIO_REMAPPED; > >>> + > >>> + /* Calculate the DUN and enforce data-unit (crypto sector) alignment. */ > >>> + dun[0] = ctx->iv_offset + sector_in_target; /* 512-byte sectors */ > >>> + if (dun[0] & ((ctx->sector_size >> SECTOR_SHIFT) - 1)) > >>> + return DM_MAPIO_KILL; > >> > >> If ctx->iv_offset is not a multiple of ctx->sector_size, this will > >> always fail. ctx->iv_offset should probably get validated in > >> inlinecrypt_ctr() > > > > ACK > > > > Yes, this assumes iv_offset is aligned to sector_size when large crypto > > sectors are used. That’s a requirement of dm-inlinecrypt semantics, and > > adding an explicit check in inlinecrypt_ctr() would make this fail earlier > > and more clearly. > > Sorry, the last response is wrong. No need to add check in inlinecrypt_ctr(). > > iv_offset is the starting offset for IVs that are generated as if the target > were > preceded by iv_offset 512-byte sectors. > > I think this concern is based on an implicit assumption that > sector_in_target is always data-unit (crypto sector) aligned. In this > target, however, sector_in_target is derived from dm_target_offset() and > is in 512-byte sectors, so it is not guaranteed to be a multiple of > (sector_size >> SECTOR_SHIFT).
sector_in_target should be guaranteed to be sector_size aligned. inlinecrypt_io_hints() sets the device logical block size to at least ctx->sector_size, and validate_hardware_logical_block_alignment() makes sure that the target starts on a logical block boundary. The block layer enforces IO to be aligned with the logical block size, so an IO that starts 7 sectors into a device with a 4096 ctx->sector_size should be impossible. -Ben
