Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE

[David Laight <david.laight.linux@gmail.com>]

On Wed, 23 Sep 2026 14:39:03 +0300
Itai Handler <itai.handler@gmail.com> wrote:

> On Wed, 23 Sep 2026, Mikulas Patocka wrote:
> > There is one important problem - the SSDs and HDDs today have 4k hardware
> > sector size.
> >
> > If you use 64k encryption sectors, the disk may write only a part of the
> > 64k sector during power failure. When you attempt to read and decrypt such
> > a sector, you get garbage.
> >
> > This is not a problem for XTS or ECB, but it is problem for CBC and most
> > other encryption modes. So, the patch should reject using larger sectors
> > with cipher modes other than XTS and ECB.  
> 
> The mechanism is real and I am not disputing it. What I would like to
> put to you is that it is not new, that this patch does not change it,
> and that documenting it fits what dm-crypt already does better than a
> new restriction would.
> 
> A torn sector is possible today
> -------------------------------
> 
> Consumer NVMe are generally shipped with 512-byte logical blocks and
> stay that way in use. Installing Linux does not change it: partitioning
> and mkfs work inside the logical blocks the drive already exposes.
> Changing the block size is a separate low-level operation - nvme format
> with a different LBA format - which is destructive, which no installer
> performs, and which a drive may not offer at all.

And you really better do 4k aligned 4k writes.
Otherwise performance and device lifetime are likely to suffer badly.

David

> 
> The machine I am writing from is one: a Samsung PM981a carrying a
> complete Ubuntu install - GPT, EFI partition, /boot, and the rest of the
> drive as LUKS over dm-crypt with LVM and ext4 on top - and it still
> reports logical_block_size = physical_block_size = 512, with no
> atomic_write_* attributes at all. The dm-crypt device on it reports a
> 512-byte logical block too.
> 
> The atomic write unit follows that format.
> nvme_configure_atomic_write() takes it from NAWUPF, and where the
> namespace does not advertise one the unit is a single logical block;
> controller-level AWUPF is explicitly ignored. So on that drive the
> guaranteed atomic unit is 512 bytes, not 4096.
> 
> Which means dm-crypt already permits, and has permitted for as long as
> sector_size has existed, exactly the situation the restriction is meant
> to prevent: a 4096-byte encryption sector spans eight device writes
> there, with seven places to tear, in any cipher mode including CBC.
> 
> What this patch changes is the maximum size. It does not change what
> any mode does when a sector is torn, and it does not make a torn sector
> possible where it was not before.
> 
> What a tear costs
> -----------------
> 
>   XTS, ECB     each cipher block is independent, so the sector decrypts
>                to a mixture of old and new plaintext - what a torn write
>                gives on an unencrypted device
>   CBC          P_i = D(C_i) ^ C_(i-1), so exactly one block, the one
>                whose predecessor is on the other side of the tear,
>                decrypts to garbage; the rest is old or new data
>   AEAD         authentication fails and the read returns an error rather
>                than data, which is the loudest and arguably the best of
>                these outcomes
>   diffusers    the whole sector decrypts to garbage; in practice only
>                reachable by writing a table by hand, since BITLK uses
>                512 or 4096
> 
> My view is that all of these are acceptable, because a sector whose
> write was not atomic is lost data in every one of them. The filesystem
> above cannot rely on a partially written block whatever comes back from
> it; the cipher mode decides whether the loss looks like stale data,
> like one corrupt block, or like an I/O error. It does not decide whether
> the data survived, because it did not.
> 
> Documenting it
> --------------
> 
> So I would rather say this plainly in
> Documentation/admin-guide/device-mapper/dm-crypt.rst than refuse
> configurations:
> 
>     An encryption sector larger than the unit the underlying device
>     writes atomically can be torn by a power failure, leaving part of
>     the sector written and part not. This is already possible with a
>     4096 byte sector on a device whose atomic write unit is 512 bytes,
>     which is the common case; a larger sector widens the window. With
>     XTS and ECB the torn sector decrypts to a mixture of old and new
>     data, as a torn write does on an unencrypted device. With chaining
>     modes the block at the tear also decrypts to garbage, and with the
>     wide-block diffusers the whole sector does. Use a large sector only
>     where losing a sector to a power failure is acceptable.
> 
> Reworded however you prefer, and I will send it as part of v3.
> 
> Rejecting modes above 4096 would be a new restriction on a hazard that
> already exists below 4096, and it would have to be revisited for every
> mode added later. It would also read oddly to a user who meets it as
> "use ECB instead", ECB being tear-tolerant but not something anyone
> should choose for disk encryption.
> 
> For what it is worth I do not think many people would meet it either
> way: cryptsetup has defaulted to xts-plain64 for plain mode, LUKS1 and
> LUKS2 for years - plain-mode, luks1-mode and luks2-keyslot-cipher in its
> configure.ac - so almost anyone asking for a large sector is on XTS
> already.
> 
> If it turns out to matter
> -------------------------
> 
> If experience shows this does need enforcing, the version worth having
> is not a mode allow-list against a constant but a comparison against
> what the device actually advertises: refuse a sector larger than
> queue_atomic_write_unit_max_bytes() for modes that cannot absorb a tear.
> That plumbing exists, and you enabled DM_TARGET_ATOMIC_WRITES for
> dm-crypt yourself last year.
> 
> It cannot go in now, because on drives like the one above it would also
> refuse 4096 and break existing tables, so it needs a deprecation path
> rather than a one-line check. That seems to me a better use of the
> effort than freezing 4096 into the code as though it were a safe size,
> and I am happy to work on it as a follow-up.
> 
> v3 will carry the documentation above, and the two changes I already owe
> this thread: dropping the dm-verity comparison, which does not apply to
> a writable target as Milan pointed out, and dropping QCE as the stated
> motivation, which Eric was right about.
> 
> Thanks,
> Itai
>