dm-crypt: allow encryption sector size up to PAGE_SIZE

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/[email protected]/
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

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.

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

index 4467f6d4b632..3a87cd4daf13 100644
@@ -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.

index 8e838530faab..045face6924b 100644
@@ -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