mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
@ 2026-09-22 12:03 Itai Handler
  2026-09-22 12:03 ` [PATCH v2 1/1] " Itai Handler
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-22 12:03 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel, Milan Broz

dm-crypt caps the "sector_size:" option at 4096 bytes.  This raises the
cap to min(PAGE_SIZE, BLK_MAX_BLOCK_SIZE), so that a kernel with a
larger page size can use a larger encryption unit.

dm-verity already bounds its data block size exactly this way -
verity_ctr() rejects "num > PAGE_SIZE" - so this is the bound dm targets
already use, not a new kind of limit.  Nothing changes where PAGE_SIZE
is 4096, the default stays 512 bytes, and no table that loads today
stops loading.

This is v2 of

  https://lore.kernel.org/dm-devel/CAFpOueRBb9y_Fgb3-c6_eFTKZR9DoAXZmxqqx0UH1Yb2rbV0RQ@mail.gmail.com/

which was NACKed in 2021.  Below is what was objected to and what this
version does about it.

1. "4096 is the smallest page size all platforms support; with a larger
   sector the device cannot be activated on a platform with a smaller
   page size, and LUKS is portable by definition." (Milan)

   Agreed, and nothing here changes that for LUKS.  Portability is a
   property of the on-disk format, so it belongs to the format layer:
   cryptsetup caps LUKS at 4096 on every path that writes a header, and
   that is untouched.  What this patch changes is the plain dm-crypt
   mapping, which has no on-disk metadata and no portability contract.

   See "Userspace" below for the one place where that split needed
   tightening, and for what is already posted.

2. "Such a patch MUST increase dm-crypt minor version." (Milan)

   Done, 1.29.0 -> 1.30.0.

3. "It doesn't come with any understanding of all the nuanced reasons
   for 4096." (Mike)

   Fair.  The 4096 was two unrelated constraints in one number:

   a) the portability rule above, which belongs to the format layer;

   b) an implementation limit of dm-crypt itself: crypt_convert_block_*()
      passes one sector to the crypto API as a single scatterlist entry
      built from one bio_vec, bio_iter_iovec() never returns more than
      PAGE_SIZE bytes, and crypt_alloc_buffer() may fall back to order-0
      pages for the write bounce buffer.  That limit is PAGE_SIZE, not
      4096.  The patch states this in a comment next to the new bound.
      Letting one sector span several vectors would lift it further,
      but that is separate work.

   BLK_MAX_BLOCK_SIZE is in the bound because crypt_io_hints() announces
   sector_size as the logical block size and blk_validate_limits()
   refuses anything above that cap.  It does not lower the limit on any
   configuration that exists today - it is 64K only with transparent
   hugepages enabled, and the only architectures with a larger PAGE_SIZE
   (hexagon and ppc44x, both 256K) cannot enable them - so the effective
   bound is PAGE_SIZE.  It is there so dm-crypt cannot announce a block
   size the block layer would reject if that ever changes.  Happy to
   drop it and cap at PAGE_SIZE alone if you prefer.

4. "The numbers are from a proprietary driver and from tcrypt, which
   says nothing about dm-crypt." (Milan)

   The numbers below are dm-crypt throughput measured with in-tree
   drivers.

5. "No random access numbers; write amplification will hurt small I/O."
   (Milan)

   It will, in the same way a filesystem block larger than the I/O size
   does.  Nothing changes unless it is asked for.  Random access numbers
   are included below.

What has changed since 2021
---------------------------

"4096 is the smallest page size all platforms support" was also the
block layer's position in 2021.  It is not any more: 47dd67532303
("block/bdev: lift block size restrictions to 64k", v6.15) raised the
block size limit to 64K on the grounds that blocksizes larger than
PAGE_SIZE are now supported.  dm-crypt's 4096 predates that.

This patch is deliberately more conservative than the block layer now
allows, because of (3b) above: it does not go past PAGE_SIZE.

Numbers
-------

arm64, 64K pages, qce (in-tree Qualcomm crypto engine), plain dm-crypt
with capi:qcom-xts(aes)-plain64 over a 1 GiB ramdisk, fio with 1 MiB
blocks, --direct=1 --iodepth=4 --ioengine=libaio, MB/s:

  sector_size   seq read   rand read   seq write   rand write
        4096        13.3        22.8        27.2         24
       65536       581         582         586          588

arm64 Cortex-A53, 64K pages, xts-aes-ce (CPU crypto extensions), plain
dm-crypt over a Samsung 970 EVO NVMe, fio with 64 KiB blocks,
--direct=1 --num_jobs=32, MB/s:

  sector_size   rand read   rand write
        4096         966          841
       65536        1054          983

This only helps where a crypto request carries a large fixed cost, that
is with drivers offloading to hardware over DMA: a 64 KiB sector
replaces sixteen descriptor setups and DMA round trips with one.  A CPU
cipher has no such cost and gains little, as the second table shows.

Userspace
---------

LUKS portability is enforced by cryptsetup, and every path that writes a
LUKS header caps the encryption sector size at 4096.  No LUKS device can
be created with a larger sector however new the kernel is.

One gap is worth stating plainly rather than leaving to be found.  LUKS2
header *validation* never bounded that field: hdr_validate_crypt_segment()
only requires it to be non-zero and 512-aligned.  A header declaring
65536 is therefore accepted as valid today, and is refused only because
dm-crypt refuses the sector size.  On a 64K-page kernel with this patch
such a header would activate.  It is a header no cryptsetup has ever
written - it takes metadata corruption or a third-party writer to
produce one - but the check belongs in the format layer either way.  A
patch doing that is posted:

  https://lore.kernel.org/cryptsetup/20260922100554.28692-1-itai.handler@gmail.com/

It is not merged yet.  It is independent of this patch in both
directions: it is a correct fix for cryptsetup on today's kernels, and
this patch does not depend on it to be correct for plain mappings.

A second cryptsetup change, lifting the cap for plain mappings only and
gating it on dm-crypt 1.30.0, will follow once the target version here
is final.  LUKS stays at 4096 in it.

Testing
-------

Built for x86_64 and for arm64 with 4K and with 64K pages.  A temporary
BUILD_BUG_ON confirmed the new bound is 4096 on x86_64, so the patch is
a no-op there, and 65536 on arm64 with 64K pages.

Booted under qemu-system-aarch64 and loaded a crypt table over /dev/ram0
with dmsetup, reading back what the kernel recorded:

  sector_size:  v1.29.0        this patch, 4K   this patch, 64K
                 (4K and 64K)
       4096     ok, 4096        ok, 4096         ok, 4096
       8192     EINVAL          EINVAL           ok, 8192
      65536     EINVAL          EINVAL           ok, 65536
      69632     ok, 4096 (!)    EINVAL           EINVAL

v1.29.0 behaves the same on both page sizes, which is the point: 8192 is
refused today even where the page size would allow it.

The last row is the %hu truncation described in the patch - 69632 wraps
to 4096, the table loads, and the device announces a 4096-byte logical
block size.  logical_block_size in sysfs followed the recorded sector
size in every accepted case.

Exercised on arm64 with a 64K page granule: plain dm-crypt with
sector_size 65536 over NVMe, block level round trip, fio random
read/write with crc32c verification, and a filesystem round trip on the
mapped device, with KASAN, lockdep and kmemleak enabled.  sector_size
512 and 4096 were run alongside as a regression check.

Itai Handler (1):
  dm-crypt: allow encryption sector size up to PAGE_SIZE

 .../admin-guide/device-mapper/dm-crypt.rst    |  8 ++++-
 drivers/md/dm-crypt.c                         | 30 +++++++++++++++----
 2 files changed, 31 insertions(+), 7 deletions(-)

base-commit: 704340f1cd0dcef829eb62f5b48ae95a2ce17bdf
-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 12:03 [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE Itai Handler
@ 2026-09-22 12:03 ` Itai Handler
  2026-09-22 12:34 ` [PATCH v2 0/1] " Itai Handler
  2026-09-22 13:07 ` Milan Broz
  2 siblings, 0 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-22 12:03 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel, Milan Broz

The "sector_size:<bytes>" option is capped at 4096 bytes, the smallest
PAGE_SIZE of any supported architecture.  A kernel with a larger
PAGE_SIZE can use a larger encryption unit, which turns several crypto
requests per page into a single one.

That only pays off when a request carries a large fixed cost, which is
the case for drivers that offload to hardware over DMA: setting the
transfer up dominates, so doing it once per 64 KiB instead of sixteen
times is worth a lot.  On an arm64 64K-page system driving the in-tree
qce driver, dm-crypt throughput rose from 13-27 MB/s to about 580 MB/s
when the encryption sector size was raised from 4096 to 65536.  A CPU
cipher has no such fixed cost and gains little: 9-16% measured with
xts-aes-ce on NVMe.

Raise the cap to min(PAGE_SIZE, BLK_MAX_BLOCK_SIZE).  dm-verity already
bounds its data block size the same way, rejecting "num > PAGE_SIZE" in
verity_ctr(), so this is the bound dm targets already use rather than a
new kind of limit.

PAGE_SIZE is the ceiling of the current conversion path: a sector is
passed to the crypto API as a single scatterlist entry, bio_iter_iovec()
never returns more than PAGE_SIZE bytes, and crypt_alloc_buffer() may
fall back to order-0 pages for the write bounce buffer.

BLK_MAX_BLOCK_SIZE is the block layer's own cap on the logical block
size that crypt_io_hints() announces.  It does not lower the limit
today - it is 64K only when transparent hugepages are enabled, and no
architecture that can enable them has a PAGE_SIZE above 64K, while
without them it is PAGE_SIZE - so the effective bound is PAGE_SIZE.  It
is in the expression so that this target cannot announce a block size
blk_validate_limits() would reject should that ever change.

Widen sector_size to unsigned int so that it can hold 65536.  That also
makes the option reject values that %hu silently truncated: an argument
of 69632 currently wraps to 4096 and is accepted as a 4096-byte sector.

Apart from that, every table accepted before is still accepted.  The
larger sizes are opt-in - the default stays 512 bytes - and nothing
changes at all where PAGE_SIZE is 4096.  A mapping above 4096 bytes can
only be activated where PAGE_SIZE allows, so it is not suitable for
portable on-disk formats such as LUKS.  Bump the target version so that
userspace can detect the new limit.

Assisted-by: LLM
Signed-off-by: Itai Handler <itai.handler@gmail.com>
---
 .../admin-guide/device-mapper/dm-crypt.rst    |  8 ++++-
 drivers/md/dm-crypt.c                         | 30 +++++++++++++++----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
index 4467f6d..250da7e 100644
--- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -153,9 +153,15 @@ integrity_key_size:<bytes>
 
 sector_size:<bytes>
     Use <bytes> as the encryption unit instead of 512 bytes sectors.
-    This option can be in range 512 - 4096 bytes and must be power of two.
+    This option can be in range 512 - PAGE_SIZE bytes, with an upper bound
+    of 65536, and must be power of two.
     Virtual device will announce this size as a minimal IO and logical sector.
 
+    An encryption unit larger than 4096 bytes can only be used on a system
+    whose PAGE_SIZE is at least that large, so such a mapping is not
+    portable across architectures and is unsuitable for portable on-disk
+    formats such as LUKS.
+
 iv_large_sectors
    IV generators will use sector number counted in <sector_size> units
    instead of default 512 bytes sectors.
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9e170de..0f087c5 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -182,7 +182,7 @@ struct crypt_config {
 	} iv_gen_private;
 	u64 iv_offset;
 	unsigned int iv_size;
-	unsigned short sector_size;
+	unsigned int sector_size;
 	unsigned char sector_shift;
 
 	union {
@@ -241,6 +241,24 @@ struct crypt_config {
 #define MAX_TAG_SIZE	480
 #define POOL_ENTRY_SIZE	512
 
+/*
+ * Largest encryption sector size that can be requested with the
+ * "sector_size:<bytes>" option.
+ *
+ * A sector is handed to the crypto API as a single scatterlist entry, so it
+ * has to be covered by one bio_vec.  bio_iter_iovec() never returns more than
+ * PAGE_SIZE bytes, and crypt_alloc_buffer() may fall back to order-0 pages
+ * for the write bounce buffer, so PAGE_SIZE is the ceiling.
+ *
+ * crypt_io_hints() announces the sector size as the logical block size, which
+ * the block layer caps at BLK_MAX_BLOCK_SIZE.  That cap is never below
+ * PAGE_SIZE in any configuration today, so it does not lower the limit; take
+ * the minimum anyway so that this target cannot announce a block size
+ * blk_validate_limits() would reject.
+ */
+#define DM_CRYPT_MAX_SECTOR_SIZE	min_t(unsigned int, PAGE_SIZE, \
+					      BLK_MAX_BLOCK_SIZE)
+
 static DEFINE_SPINLOCK(dm_crypt_clients_lock);
 static unsigned int dm_crypt_clients_n;
 static volatile unsigned long dm_crypt_pages_per_client;
@@ -3134,9 +3152,9 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 			}
 			cc->key_mac_size = val;
 			set_bit(CRYPT_KEY_MAC_SIZE_SET, &cc->cipher_flags);
-		} else if (sscanf(opt_string, "sector_size:%hu%c", &cc->sector_size, &dummy) == 1) {
+		} else if (sscanf(opt_string, "sector_size:%u%c", &cc->sector_size, &dummy) == 1) {
 			if (cc->sector_size < (1 << SECTOR_SHIFT) ||
-			    cc->sector_size > 4096 ||
+			    cc->sector_size > DM_CRYPT_MAX_SECTOR_SIZE ||
 			    (cc->sector_size & (cc->sector_size - 1))) {
 				ti->error = "Invalid feature value for sector_size";
 				return -EINVAL;
@@ -3556,7 +3574,7 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
 			if (cc->used_tag_size)
 				DMEMIT(" integrity:%u:%s", cc->used_tag_size, cc->cipher_auth);
 			if (cc->sector_size != (1 << SECTOR_SHIFT))
-				DMEMIT(" sector_size:%d", cc->sector_size);
+				DMEMIT(" sector_size:%u", cc->sector_size);
 			if (test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags))
 				DMEMIT(" iv_large_sectors");
 			if (test_bit(CRYPT_KEY_MAC_SIZE_SET, &cc->cipher_flags))
@@ -3582,7 +3600,7 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(",integrity_tag_size=%u,cipher_auth=%s",
 			       cc->used_tag_size, cc->cipher_auth);
 		if (cc->sector_size != (1 << SECTOR_SHIFT))
-			DMEMIT(",sector_size=%d", cc->sector_size);
+			DMEMIT(",sector_size=%u", cc->sector_size);
 		if (cc->cipher_string)
 			DMEMIT(",cipher_string=%s", cc->cipher_string);
 
@@ -3700,7 +3718,7 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 29, 0},
+	.version = {1, 30, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 12:03 [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE Itai Handler
  2026-09-22 12:03 ` [PATCH v2 1/1] " Itai Handler
@ 2026-09-22 12:34 ` Itai Handler
  2026-09-22 13:07 ` Milan Broz
  2 siblings, 0 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-22 12:34 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel, Milan Broz

On Tue, Sep 22, 2026 at 3:04 PM Itai Handler <itai.handler@gmail.com> wrote:

> A patch doing that is posted:
>
>   https://lore.kernel.org/cryptsetup/20260922100554.28692-1-itai.handler@gmail.com/
>
> It is not merged yet.

That cryptsetup patch has since been declined, four minutes after I sent
this series. Correcting the record here rather than leaving the
reference dangling:

  https://lore.kernel.org/cryptsetup/d3823e0b-3387-4b3a-b5e8-a3d5f266308a@redhat.com/

Ondrej Kozina's reasoning is that the missing upper bound in LUKS2
header validation is deliberate: bounding it would stop an older
cryptsetup from even dumping a header that a newer dm-crypt might
support, and the corruption case is already covered by the LUKS2 header
checksum. Both points are correct, and I have withdrawn the patch.

It does not change the position of this one, and it is worth spelling
out why:

- cryptsetup caps the encryption sector size at 4096 on every path that
  *writes* a LUKS header, and this patch does not touch that. No tool
  creates a LUKS device above 4096, so the portability of what
  cryptsetup produces is unchanged.

- A LUKS header declaring more than 4096 can therefore only come from a
  deliberate edit, and as Ondrej notes LUKS2 metadata does not protect
  against deliberate harmful edits in general - changing 512 to 4096
  breaks a device just as effectively.

So the paragraph above should be read as: this is a case that requires
hand-editing metadata, upstream cryptsetup considers guarding it the
wrong trade-off, and I agree.

Two remarks from that thread bear on this patch. Neither is an ack of
it, but both are worth having in this thread:

  "There may be support for larger sectors in future dm-crypt."

  "Since dm-crypt constructor currently stops you from activating such
   device (w/ sector_size > 4096), it's ok."

That is, cryptsetup deliberately leaves this limit to the dm-crypt
constructor. This patch keeps it exactly there. It only changes the
limit from a fixed 4096 to PAGE_SIZE, which is the bound dm-verity
already applies to its own block size.

Itai

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 12:03 [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE Itai Handler
  2026-09-22 12:03 ` [PATCH v2 1/1] " Itai Handler
  2026-09-22 12:34 ` [PATCH v2 0/1] " Itai Handler
@ 2026-09-22 13:07 ` Milan Broz
  2026-09-22 13:49   ` Itai Handler
  2 siblings, 1 reply; 13+ messages in thread
From: Milan Broz @ 2026-09-22 13:07 UTC (permalink / raw)
  To: Itai Handler, Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel, Ondrej Kozina

On 9/22/26 2:03 PM, Itai Handler wrote:
...> Exercised on arm64 with a 64K page granule: plain dm-crypt with
> sector_size 65536 over NVMe, block level round trip, fio random

Could you please point me to at least one consumer-grade NVMe drive that
supports a 64k sector size?

Allowing a bigger sector size in dm-crypt means you can end up with
partial dm-crypt sector writes (after power fail), which opens another
can of worms.

Your dm-verity argument does not apply here; it is a read-only target.

Also, arguments based on cryptsetup/LUKS2 do not make much sense for
kernel code. They must be compatible and work together, but dm-crypt
can be used without any userspace validation, so all limits must be
checked in the kernel.

LUKS2 limits the sector size to 4k for multiplatform compatibility.

dm-crypt itself can support bigger sectors, but I just do not see much
use for it with common NVMe drives or other storage.

Milan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 13:07 ` Milan Broz
@ 2026-09-22 13:49   ` Itai Handler
  2026-09-22 21:34     ` Eric Biggers
  0 siblings, 1 reply; 13+ messages in thread
From: Itai Handler @ 2026-09-22 13:49 UTC (permalink / raw)
  To: Milan Broz, Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel

On 22/09/2026 16:07, Milan Broz wrote:
> Could you please point me to at least one consumer-grade NVMe drive that
> supports a 64k sector size?

None that I know of, and none is needed - I think my cover letter invited
that reading, sorry.

The encryption sector size is dm-crypt's crypto chunking unit, not a
property required of the backing device. dm-crypt never validates it
against the device. The only use of bdev_logical_block_size() in the
target is

	sector_align = max(bdev_logical_block_size(cc->dev->bdev),
			   (unsigned)cc->sector_size);

in get_max_request_sectors(), which is request alignment, not a check.
A mapping whose encryption sector exceeds the drive's block size is
already the normal case - sector_size:4096 on a 512e drive is exactly
that.

The drive is also not where the gain comes from; see the last point.

> Allowing a bigger sector size in dm-crypt means you can end up with
> partial dm-crypt sector writes (after power fail), which opens another
> can of worms.

This is the objection I take most seriously, and you are right that it
is real.

It is not a new hazard though - it is the existing one scaled. The same
tearing is possible whenever the encryption sector exceeds the drive's
atomic unit, which is every sector_size:4096 mapping on a 512e drive.
cryptsetup already documents precisely this for --sector-size:

  "Note that using a sector size larger than the underlying storage
   device's physical sector size may result in data corruption during
   unexpected power failures.  A power failure during write operations
   may result in only partial completion of the encryption sector
   write, leaving encrypted data in an inconsistent state that cannot
   be properly decrypted."

What a larger sector changes is the width of the window, not its
existence, and only for a mapping that asks for it - the default stays
512 bytes.

For the common case the failure mode is also unchanged: with xts(aes)
each 16-byte block carries its own tweak, so a torn write leaves a
mixture of old and new plaintext at that granularity rather than
destroying the sector. The modes that would be worse are already
restricted - crypt_iv_lmk_ctr() and crypt_iv_tcw_ctr() reject anything
but 512 bytes. The exception I am aware of is the Elephant diffuser,
which does diffuse across the whole sector; it is reachable only by
hand-writing a table, since cryptsetup's BITLK support allows 512 and
4096 only, but I mention it rather than leave you to find it.

I would rather document this than leave it implicit. If the patch is
worth pursuing at all, I will add to
Documentation/admin-guide/device-mapper/dm-crypt.rst:

    An encryption sector larger than the atomic write unit of the
    underlying device 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 512e device; a larger sector widens the
    window. Use one only where that is acceptable.

Reworded however you prefer.

> Your dm-verity argument does not apply here; it is a read-only target.

You are right, and I overreached. dm-verity shows only that PAGE_SIZE is
an accepted bound for how much data one target request may cover; it
says nothing about write atomicity, which is the part that matters here.
I will drop the comparison.

> Also, arguments based on cryptsetup/LUKS2 do not make much sense for
> kernel code. They must be compatible and work together, but dm-crypt
> can be used without any userspace validation, so all limits must be
> checked in the kernel.

Agreed, and that is what the patch does. The limit is enforced in
crypt_ctr_optional():

	cc->sector_size > DM_CRYPT_MAX_SECTOR_SIZE

with DM_CRYPT_MAX_SECTOR_SIZE = min(PAGE_SIZE, BLK_MAX_BLOCK_SIZE). It
needs no userspace cooperation: dmsetup and a raw ioctl are bound by it
exactly as cryptsetup is, and on a 4k-page kernel it is still 4096, so
nothing changes there at all.

The LUKS2 4k rule is a different kind of thing. It is a property of an
on-disk format the kernel knows nothing about, so it cannot be enforced
here and I am not proposing that it should be - it stays in cryptsetup,
untouched. I raised it only to answer the portability objection from the
v1 thread, not as an argument for kernel behaviour. Ondrej made the same
point back to me on the cryptsetup list earlier today, and he was right:

  https://lore.kernel.org/cryptsetup/d3823e0b-3387-4b3a-b5e8-a3d5f266308a@redhat.com/

> LUKS2 limits the sector size to 4k for multiplatform compatibility.

Yes, and nothing in this patch changes that.

> dm-crypt itself can support bigger sectors, but I just do not see much
> use for it with common NVMe drives or other storage.

Storage is not the use case - crypto offload is.

Where the cipher is a hardware engine driven over DMA, the per-request
cost is a descriptor setup and a round trip, and that cost dominates.
Making the request sixteen times larger amortises it. With the in-tree
qce driver on an arm64 64k-page board, plain dm-crypt over a ramdisk,
MB/s:

  sector_size   seq read   rand read   seq write   rand write
        4096        13.3        22.8        27.2         24
       65536       581         582         586          588

The ramdisk is deliberate - it keeps the storage out of the measurement,
because the storage is not what is being fixed.

That also answers the NVMe half of the question, and I should have made
it the main point rather than the ramdisk. A common consumer NVMe drive
delivers something in the region of 1-3 GB/s. Driving qce with a
4096-byte sector, dm-crypt manages 13-27 MB/s, so behind any such drive
the crypto engine is the limit rather than the drive, by about two
orders of magnitude. At 65536 bytes it reaches roughly 580 MB/s and the
engine is still the limit. The ramdisk figures are therefore a
reasonable predictor of what the same setup does on ordinary storage:
nothing unusual is being asked of the drive, only that the cipher is
offloaded to an engine reached over DMA. The drive can be entirely
common - it is the accelerator behind it that is currently being driven
inefficiently.

The other measurement in the cover letter is the NVMe one, and it is a
different board: an arm64 Cortex-A53, also 64k pages, but with the CPU
cipher xts-aes-ce over a Samsung 970 EVO. There the gain is 9-16%,
which matches your intuition - for a CPU cipher there is very little in
this, and the drive is not the limit either.

So I would put the case as: it is not about the drive at all. It is for
systems that have a crypto accelerator and a 64k page granule, where
dm-crypt currently leaves most of the engine's throughput unused - and
the storage behind them can be an entirely ordinary NVMe. If that is
still too narrow to justify the option, that is a fair conclusion to
reach, but it is the case I am asking about, and the numbers are with
an in-tree driver.

Thanks for taking the time on this.

Itai

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 13:49   ` Itai Handler
@ 2026-09-22 21:34     ` Eric Biggers
  2026-09-23  7:29       ` Itai Handler
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2026-09-22 21:34 UTC (permalink / raw)
  To: Itai Handler
  Cc: Milan Broz, Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel

On Tue, Sep 22, 2026 at 04:49:45PM +0300, Itai Handler wrote:
> Storage is not the use case - crypto offload is.
> 
> Where the cipher is a hardware engine driven over DMA, the per-request
> cost is a descriptor setup and a round trip, and that cost dominates.
> Making the request sixteen times larger amortises it. With the in-tree
> qce driver on an arm64 64k-page board, plain dm-crypt over a ramdisk,
> MB/s:

That isn't a real use case, though.  Using the QCE driver is *much*
slower than just using the encryption on the CPU, even on long messages.
That's been established in many previous discussions and is now even
admitted by the maintainers of the QCE driver.  The QCE driver even
(incredibly) uses more CPU time than just doing the crypto on the CPU,
due to all its driver, scheduling, and IRQ overhead.  The pending
patches to add BAM locking will make it even slower.

The fix is to not use QCE.  (And also ensure that
CONFIG_CRYPTO_AES_ARM64_CE_BLK=y is set, to get the ARMv8 CE accelerated
code.  But even on legacy CPUs without ARMv8 CE, QCE is still slower.)

Unsurprisingly, the numbers in this patch's cover letter show this as
well, with much higher throughput reported for the CPU-based encryption.

(Also note that QCE is currently marked as BROKEN upstream.)

I really do not think dm-crypt should accept changes to further
accommodate obsolete and problematic external crypto engines like this.

- Eric

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-22 21:34     ` Eric Biggers
@ 2026-09-23  7:29       ` Itai Handler
  2026-09-23  9:58         ` Mikulas Patocka
  0 siblings, 1 reply; 13+ messages in thread
From: Itai Handler @ 2026-09-23  7:29 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Milan Broz, Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel

On Wed, Sep 23, 2026 at 12:34 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Sep 22, 2026 at 04:49:45PM +0300, Itai Handler wrote:
> > Storage is not the use case - crypto offload is.
> >
> > Where the cipher is a hardware engine driven over DMA, the per-request
> > cost is a descriptor setup and a round trip, and that cost dominates.
> > Making the request sixteen times larger amortises it. With the in-tree
> > qce driver on an arm64 64k-page board, plain dm-crypt over a ramdisk,
> > MB/s:
>
> That isn't a real use case, though.  Using the QCE driver is *much*
> slower than just using the encryption on the CPU, even on long messages.
> That's been established in many previous discussions and is now even
> admitted by the maintainers of the QCE driver.  The QCE driver even
> (incredibly) uses more CPU time than just doing the crypto on the CPU,
> due to all its driver, scheduling, and IRQ overhead.  The pending
> patches to add BAM locking will make it even slower.
>
> The fix is to not use QCE.  (And also ensure that
> CONFIG_CRYPTO_AES_ARM64_CE_BLK=y is set, to get the ARMv8 CE accelerated
> code.  But even on legacy CPUs without ARMv8 CE, QCE is still slower.)
>
> Unsurprisingly, the numbers in this patch's cover letter show this as
> well, with much higher throughput reported for the CPU-based encryption.
>
> (Also note that QCE is currently marked as BROKEN upstream.)
>
> I really do not think dm-crypt should accept changes to further
> accommodate obsolete and problematic external crypto engines like this.

Thanks for the detailed feedback.

I agree that QCE is not a good example to use as the main motivation for
this change if its driver overhead makes it slower, and even more so if
the driver consumes more CPU time than the software implementation. I
will therefore not argue that QCE itself justifies changing dm-crypt.

However, I don't think this necessarily establishes that the generic
dm-crypt change is unnecessary.

One point regarding the suggested ARMv8 Crypto Extensions implementation:
PAGE_SIZE and the presence of ARMv8 Crypto Extensions are independent
properties. In particular, an ARM64 system can use 64 KiB pages without
having ARMv8 Crypto Extensions. Therefore, the fact that AES-CE is faster
than QCE on CPUs which support it does not apply to all systems where the
proposed change would be possible.

There is also a more general reason why hardware crypto offload can be
useful even when its raw throughput is lower than CPU-based encryption.
Offloading encryption can leave CPU cycles available for the application,
filesystem, networking, or other work. Of course, this only makes sense
when the accelerator and its driver have reasonable overhead; I agree that
QCE is a poor example if its driver overhead outweighs this benefit.

My main question with the patch is therefore independent of QCE:

Is there a reason for dm-crypt itself to impose a fixed 4096-byte
encryption-unit limit, rather than allowing an encryption unit up to the
size that its existing I/O conversion path can safely handle?

The patch does not make larger sectors the default, and it does not
remove the kernel-side validation. The default remains 512 bytes, and a
larger sector must be explicitly requested. The proposed upper bound is
the smaller of PAGE_SIZE and BLK_MAX_BLOCK_SIZE, so configurations that
cannot represent such an I/O unit are still rejected.

The motivation for using PAGE_SIZE as the upper bound is also not tied
to any particular crypto accelerator. The current dm-crypt conversion
path processes the encryption unit through the bio/scatterlist
representation, and the relevant bio vector cannot represent more than
PAGE_SIZE in a single entry. Consequently, PAGE_SIZE is already a
natural upper bound for the current implementation.

The performance measurements with QCE were intended to demonstrate the
kind of workload where reducing the number of crypto submissions can
matter, rather than to claim that QCE is itself a desirable accelerator.
I agree that I should find a better hardware-accelerator example before
using those measurements as evidence for the patch.

I also don't intend this change to imply that larger encryption sectors
are appropriate for LUKS or other portable on-disk formats. That is a
separate userspace/on-disk-format consideration. The kernel would still
enforce the limits of the dm-crypt implementation itself.

So I would frame the patch more narrowly: this is an attempt to remove
an apparently arbitrary 4096-byte limitation from dm-crypt and allow
larger encryption units on systems where PAGE_SIZE and the block-layer
limits permit them, without changing the default behavior.

If there is a specific correctness, security, or architectural reason
why dm-crypt should retain 4096 bytes as an absolute limit even on
systems with larger PAGE_SIZE, I would be very interested to understand
it. Otherwise, I think the question is better evaluated independently
of whether QCE is a suitable accelerator.

Thanks,
Itai

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23  7:29       ` Itai Handler
@ 2026-09-23  9:58         ` Mikulas Patocka
  2026-09-23 11:39           ` Itai Handler
  0 siblings, 1 reply; 13+ messages in thread
From: Mikulas Patocka @ 2026-09-23  9:58 UTC (permalink / raw)
  To: Itai Handler
  Cc: Eric Biggers, Milan Broz, Alasdair Kergon, Mike Snitzer,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel



On Wed, 23 Sep 2026, Itai Handler wrote:

> If there is a specific correctness, security, or architectural reason
> why dm-crypt should retain 4096 bytes as an absolute limit even on
> systems with larger PAGE_SIZE, I would be very interested to understand
> it. Otherwise, I think the question is better evaluated independently
> of whether QCE is a suitable accelerator.
> 
> Thanks,
> Itai

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.

Mikulas


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23  9:58         ` Mikulas Patocka
@ 2026-09-23 11:39           ` Itai Handler
  2026-09-23 11:59             ` Mikulas Patocka
  2026-09-23 15:28             ` David Laight
  0 siblings, 2 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-23 11:39 UTC (permalink / raw)
  To: Mikulas Patocka, Eric Biggers, Milan Broz, Alasdair Kergon,
	Mike Snitzer, Benjamin Marzinski, Jonathan Corbet, Shuah Khan,
	Randy Dunlap, dm-devel, linux-doc, linux-kernel

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.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23 11:39           ` Itai Handler
@ 2026-09-23 11:59             ` Mikulas Patocka
  2026-09-23 12:43               ` Itai Handler
  2026-09-23 15:28             ` David Laight
  1 sibling, 1 reply; 13+ messages in thread
From: Mikulas Patocka @ 2026-09-23 11:59 UTC (permalink / raw)
  To: Itai Handler
  Cc: Eric Biggers, Milan Broz, Alasdair Kergon, Mike Snitzer,
	Benjamin Marzinski, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	dm-devel, linux-doc, linux-kernel



On Wed, 23 Sep 2026, Itai Handler wrote:

> 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.
> 
> 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.

I did some benchmarking some times ago - and all the SSDs that I tested 
had worse IOPS for sub-4k writes than for 4k-aligned writes. This means 
that the SSDs have 4k sectors internally and do read-modify-write cycle 
for sub-4k writes.

The SATA standard doesn't specify well what 'physical block size' should 
mean on SSD - the result is that some SATA SSDs report physical block size 
512 and some 4k.

The SSDs have remapping table that maps logical blocks to locations in the 
flash chips - and RAM is expensive, so I doubt that any SSD vendor would 
put 8 times more RAM on the SSD in order to map 512-byte sectors 
individually.

Mikulas


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23 11:59             ` Mikulas Patocka
@ 2026-09-23 12:43               ` Itai Handler
  0 siblings, 0 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-23 12:43 UTC (permalink / raw)
  To: Mikulas Patocka, Eric Biggers, Milan Broz, Alasdair Kergon,
	Mike Snitzer, Benjamin Marzinski, Jonathan Corbet, Shuah Khan,
	Randy Dunlap, dm-devel, linux-doc, linux-kernel

On Wed, 23 Sep 2026, Mikulas Patocka wrote:
> I did some benchmarking some times ago - and all the SSDs that I tested
> had worse IOPS for sub-4k writes than for 4k-aligned writes. This means
> that the SSDs have 4k sectors internally and do read-modify-write cycle
> for sub-4k writes.
>
> The SSDs have remapping table that maps logical blocks to locations in the
> flash chips - and RAM is expensive, so I doubt that any SSD vendor would
> put 8 times more RAM on the SSD in order to map 512-byte sectors
> individually.

I accept the measurement and what you conclude from it about the mapping
table. A drive that is slower for sub-4k writes is doing
read-modify-write, and a 4k mapping granularity is the obvious reason.

What I do not think follows is that a 4k write is therefore atomic
across a power failure. Those are two different properties:

 - the mapping granularity determines how much has to be rewritten for a
   partial update, which is what the IOPS measurement shows;

 - atomicity across power loss depends on whether the commit is
   all-or-nothing - whether the NAND program can be interrupted part way
   through a page, and whether the mapping update itself survives.
   Without power-loss protection there is nothing holding either up, and
   consumer drives do not have it.

That is what AWUPF and NAWUPF exist to express, and it is why the value
is advertised separately rather than derived from the geometry.

The kernel takes that position too, and more firmly than I had been
arguing. nvme_configure_atomic_write() accepts only NAWUPF, explicitly
ignores controller-level AWUPF, and where neither is advertised falls
back to a single logical block. Then in nvme_update_disk_info():

	/*
	 * Linux filesystems assume writing a single physical block is
	 * an atomic operation. Hence limit the physical block size to the
	 * value of the Atomic Write Unit Power Fail parameter.
	 */
	lim->logical_block_size = bs;
	lim->physical_block_size = min(phys_bs, atomic_bs);

So the block layer will not even report a 4096-byte physical block
unless the drive advertised that it can write 4096 bytes atomically,
specifically so that nothing above it draws the conclusion we are being
asked to draw here. On the drive in front of me that yields
physical_block_size = 512 - the kernel declining to infer atomicity from
geometry, rather than the drive being unusual.

The empirical work agrees: the FAST'13 power-fault study found shorn
writes - partial writes inside a single page - along with metadata
corruption and worse, on most of the consumer devices tested. Whatever
the mapping granularity was on those drives, 4k writes were not atomic
on them.

So my difficulty is not that 4096 is too small a limit. It is that 4096
is not a boundary between safe and unsafe: below it there is no
guarantee either, unless the drive advertises one, and few do.

What this patch does, and does not do
-------------------------------------

It raises the maximum sector size. It does not change what any cipher
mode does when a sector is torn, and it does not make a torn sector
possible where it was not possible before - a 4096-byte sector on a
drive that advertises no atomic write unit already spans eight of them.

What a tear costs, in full, so that nothing here is hidden:

  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
  diffusers    the whole sector decrypts to garbage; in practice only
               reachable by writing a table by hand, since BITLK uses
               512 or 4096

A sector whose write was not atomic is lost data in all of these. The
filesystem above cannot rely on a partially written block whatever comes
back from it. The cipher mode decides what the loss looks like - stale
data, one corrupt block, or an I/O error - not whether the data
survived, because it did not.

So I would rather document it than refuse configurations, in
Documentation/admin-guide/device-mapper/dm-crypt.rst:

    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. Unless the device advertises an
    atomic write unit, that unit is a single logical block, so this is
    already possible at 4096 bytes; 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 like, and I will put it in v3.

If enforcement is wanted later, the version worth having is a comparison
against what the device advertises rather than against a constant:
refuse a sector larger than queue_atomic_write_unit_max_bytes() for
modes that cannot absorb a tear. That cannot land as it stands, because
with NAWUPF unset it would also refuse 4096 and break existing tables,
so it needs a deprecation path rather than a one-line check. You enabled
DM_TARGET_ATOMIC_WRITES for dm-crypt last year, so the plumbing is
there, and I am happy to work on it separately.

Hardcoding 4096 seems to me to give up the guarantee the block layer is
careful not to give, while looking like it provides one - and to do it
for a combination almost nobody uses, since cryptsetup has defaulted to
xts-plain64 for plain mode, LUKS1 and LUKS2 for years.

I realise I am pushing back on a maintainer here, and I will not keep
doing it indefinitely. But I would rather make the case once, properly,
than change the patch while believing the line is drawn in the wrong
place.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23 11:39           ` Itai Handler
  2026-09-23 11:59             ` Mikulas Patocka
@ 2026-09-23 15:28             ` David Laight
  2026-09-23 15:42               ` Itai Handler
  1 sibling, 1 reply; 13+ messages in thread
From: David Laight @ 2026-09-23 15:28 UTC (permalink / raw)
  To: Itai Handler
  Cc: Mikulas Patocka, Eric Biggers, Milan Broz, Alasdair Kergon,
	Mike Snitzer, Benjamin Marzinski, Jonathan Corbet, Shuah Khan,
	Randy Dunlap, dm-devel, linux-doc, linux-kernel

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
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE
  2026-09-23 15:28             ` David Laight
@ 2026-09-23 15:42               ` Itai Handler
  0 siblings, 0 replies; 13+ messages in thread
From: Itai Handler @ 2026-09-23 15:42 UTC (permalink / raw)
  To: David Laight
  Cc: Mikulas Patocka, Eric Biggers, Milan Broz, Alasdair Kergon,
	Mike Snitzer, Benjamin Marzinski, Jonathan Corbet, Shuah Khan,
	Randy Dunlap, dm-devel, linux-doc, linux-kernel

On Wed, Sep 23, 2026 at 6:28 PM David Laight
<david.laight.linux@gmail.com> wrote:

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

Agreed. My point about the 512-byte logical/atomic unit was only about
what the device guarantees, not about what I/O size should normally be
used.

In particular, I don't think the fact that a device reports 512-byte
logical blocks is a reason to use 512-byte I/O. Filesystems and
applications should still normally issue appropriately aligned 4K (or
larger) I/O where that matches the device's characteristics.

The distinction I was trying to make is between the device's I/O
granularity/performance characteristics and its guaranteed atomic write
unit. The former may well be 4K or larger even when the latter is only
512 bytes (or is not advertised at all).

So I agree with your point, but I don't think it changes the argument
about the dm-crypt sector-size limit: a 4096-byte dm-crypt encryption
sector can already span multiple device-level atomic units, and the
proposed change only increases the maximum size when explicitly
requested.

Thanks,
Itai

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-09-23 15:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 12:03 [PATCH v2 0/1] dm-crypt: allow encryption sector size up to PAGE_SIZE Itai Handler
2026-09-22 12:03 ` [PATCH v2 1/1] " Itai Handler
2026-09-22 12:34 ` [PATCH v2 0/1] " Itai Handler
2026-09-22 13:07 ` Milan Broz
2026-09-22 13:49   ` Itai Handler
2026-09-22 21:34     ` Eric Biggers
2026-09-23  7:29       ` Itai Handler
2026-09-23  9:58         ` Mikulas Patocka
2026-09-23 11:39           ` Itai Handler
2026-09-23 11:59             ` Mikulas Patocka
2026-09-23 12:43               ` Itai Handler
2026-09-23 15:28             ` David Laight
2026-09-23 15:42               ` Itai Handler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®