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

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

[Itai Handler]

dm-crypt caps the "sector_size:" option at 4096 bytes.  Commit
8f0009a22517 ("dm crypt: optionally support larger encryption sector
size") gives the reason: the sector has to fit the page limit, so the
cap was set to the smallest page size any architecture has.  This
resolves that same rule against the kernel being built instead, so a
kernel with larger pages is not held to another architecture's limit.

v2: https://lore.kernel.org/dm-devel/20260922120330.127262-1-itai.handler@gmail.com/
v1: https://lore.kernel.org/dm-devel/CAFpOueRBb9y_Fgb3-c6_eFTKZR9DoAXZmxqqx0UH1Yb2rbV0RQ@mail.gmail.com/

Changes since v2
----------------

- Dropped the dm-verity comparison.  Milan pointed out that dm-verity is
  read-only, so its PAGE_SIZE bound says nothing about a writable
  target.  It was never load-bearing; it is just gone.

- Dropped qce as the motivation.  Eric was right that it is slower than
  the CPU cipher and marked BROKEN, so numbers from it argue for
  nothing.  The commit message now works from the reason the limit was
  given when it was introduced, and mentions DMA offload only as a
  possibility that depends on the driver.

- Documented in dm-crypt.rst that a sector larger than the device's
  atomic write unit can be torn by a power failure, and what that costs
  in each cipher mode, AEAD included.

- Used MIN_T() rather than min_t() for the bound, so it stays usable as
  a constant expression; min_t() expands to a statement expression.

- Dropped the claim that the documented range tops out at 65536.  It
  does not: without transparent hugepages BLK_MAX_BLOCK_SIZE is
  PAGE_SIZE, so on a 256K-page configuration the bound is PAGE_SIZE.
  The documentation now states the rule rather than a number.

On refusing other cipher modes
------------------------------

Mikulas asked for sector sizes above 4096 to be refused for modes other
than XTS and ECB.  This version does not do that.  Since the decision is
his rather than mine, the patch for it is written and tested and will go
out on request.

What gives me pause is only this:

- An allow-list of two mode names has to be revisited whenever a mode is
  added, by someone who remembers why it is there.

- It gives 4096 a safety meaning that the block layer declines to give
  it.  nvme_configure_atomic_write() trusts only NAWUPF and otherwise
  falls back to a single logical block, and nvme_update_disk_info() caps
  physical_block_size by atomic_bs precisely so nothing above it assumes
  a physical block is written atomically.  A drive advertising no atomic
  write unit, which is the common case, can already tear a 4096-byte
  sector.

- Selecting on tear-tolerance alone admits ecb(), which nobody should
  use for disk encryption, while refusing cbc().

Against that, his point that the modes differ in how badly a tear hurts
is right, and v3 writes it down per mode in dm-crypt.rst - which is the
part I should have done in v2 instead of only arguing.

On a 64K-page kernel the restriction refuses aes-cbc-essiv:sha256 at
65536 while leaving XTS and ECB working, including ESSIV-wrapped XTS,
and changes nothing at or below 4096.  Either form works - patch 2/2 of
a v4, or a standalone follow-up - whichever is preferred.

Answering Milan on NVMe
-----------------------

"Which NVMe supports 64K sectors?" - none, and none has to.  The
encryption sector is dm-crypt's own chunking unit, not something the
backing device has to support.  bdev_logical_block_size() appears once
in the whole target, inside a max() that aligns the I/O size.  The
mapped device announces the larger block upward through
dm_stack_bs_limits(); the device underneath keeps its own.  In the
testing below the backing device is /dev/ram0 with a 512-byte logical
block, carrying a 65536-byte encryption sector.

Testing
-------

Built for x86_64 and for arm64 with 4K and with 64K pages; no new
warnings.  A temporary static_assert confirmed the bound is 4096 on
x86_64 and on arm64/4K - so the patch is a no-op on both - and 65536 on
arm64 with 64K pages.

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

  sector_size   v1.29.0        v1.30.0, 4K   v1.30.0, 64K
       512      ok             ok            ok
      1024      ok             ok            ok
      4096      ok             ok            ok
     65536      EINVAL         EINVAL        ok, lbs 65536
     69632      ok, 4096 (!)   EINVAL        EINVAL
    131072      EINVAL         EINVAL        EINVAL
         0      EINVAL         EINVAL        EINVAL

Every row was run with aes-xts-plain64, aes-cbc-essiv:sha256 and
aes-ecb, and the result was the same for all three: this version refuses
nothing on the basis of the cipher.

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

Also exercised at 65536 on 64K pages: aes-xts-essiv:sha256, the capi:
form of xts(aes), and iv_large_sectors, which is where sector_shift
reaches 7.  256 KiB round-tripped through the mapped device
byte-identical at sector_size 65536 and 4096, with the ciphertext on
/dev/ram0 confirmed to differ from the plaintext.

Not covered: dm-integrity caps its block_size at 4096, so the AEAD and
integrity-tag paths cannot be driven above that today and were tested
only at 4096 and below.

Userspace
---------

Nothing is needed for this patch.  cryptsetup caps the sector size at
4096 on every path that writes a LUKS header, so no LUKS device gains a
larger sector however new the kernel is; on-disk format policy stays in
userspace, which was Milan's point in v2 and is why no LUKS2 change is
proposed here.  The LUKS2 header-validation patch posted alongside v2
was withdrawn - Ondrej was right that the missing bound is deliberate.

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

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


base-commit: c0df022cb0fa2bbee74bd9b90c66790bcd4e3050
-- 
2.34.1

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

[Itai Handler]

The "sector_size:<bytes>" option is capped at 4096 bytes.  The reason is
stated in commit 8f0009a22517 ("dm crypt: optionally support larger
encryption sector size"): "the maximal IO must fit into the page limit,
so the limit is set to the minimal page size possible (4096 bytes)."

The rule is right; only the way it is resolved is not.  The page limit
it refers to is a property of the kernel that is running, but it was
written as the smallest page size of any architecture, so a kernel with
larger pages is held to a limit that belongs to a different one.  The
block layer has since stopped doing that for its own block size, in
commit 47dd67532303 ("block/bdev: lift block size restrictions to 64k"),
and dm-crypt is now the stricter of the two.

Apply the same rule to the kernel being built: raise the cap to
min(PAGE_SIZE, BLK_MAX_BLOCK_SIZE).

PAGE_SIZE is still the page limit the original commit meant.  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 caps the logical block size that crypt_io_hints()
announces.  It is PAGE_SIZE without transparent hugepages, and 64K with
them, which no architecture having a PAGE_SIZE above 64K can enable - so
it does not lower the bound on any configuration today.  It is in the
expression so that this target cannot announce a block size
blk_validate_limits() would reject.

A larger unit also turns several crypto requests per page into one.
Whether that is worth anything depends on the driver: for a CPU cipher
the per-request cost is small, and it only pays off where a request
carries a large fixed cost, as when the cipher is offloaded over DMA.

Widen sector_size to unsigned int so that it can hold a sector larger
than 65535.  That also makes the option reject an argument of 69632,
which %hu truncates to 4096 and accepts as a 4096-byte sector.

Document that an encryption sector larger than the unit the device
writes atomically can be torn by a power failure, and what each cipher
mode does when that happens.

The maximum stays 4096 wherever PAGE_SIZE is 4096, the default stays 512
bytes, and every table using a size from 512 to 4096 behaves as it did.
The one behavioural change is the truncation above: an argument that
wrapped into range is now rejected rather than silently accepted.  A
mapping larger than 4096 bytes can only be activated where PAGE_SIZE
allows, so it is not suitable for portable on-disk formats.  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    | 20 ++++++++++++-
 drivers/md/dm-crypt.c                         | 30 +++++++++++++++----
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
index 4467f6d4b632..3a87cd4daf13 100644
--- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -153,9 +153,27 @@ 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, further limited by
+    the block layer's maximum block size, 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.
+
+    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. A device that advertises no atomic write unit
+    gives no such guarantee beyond 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, with AEAD the
+    whole sector fails authentication, and with the wide-block diffusers
+    the whole sector decrypts to garbage. Use a large sector only where
+    losing a sector to a power failure is acceptable.
+
 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 8e838530faab..045face6924b 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;
@@ -3137,9 +3155,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;
@@ -3559,7 +3577,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))
@@ -3585,7 +3603,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);
 
@@ -3703,7 +3721,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