Repository navigation
block, ext4: allow encryption with a block size larger than the page size - #2827
Open
vfsci-bot[bot] wants to merge 13 commits into
Open
vfsci-bot[bot] wants to merge 13 commits into
vfsci-bot[bot] wants to merge 13 commits into
Conversation
Encryption is not supported when a filesystem's block size is larger than the page size, and commit 709f0f1 ("ext4: add checks for large folio incompatibilities when BS > PS") added a check to refuse mounting such a filesystem. However, it is also possible to enable the "encrypt" feature on a filesystem that is already mounted (with tune2fs -O encrypt). Once that happens it won't be possible to mount the filesystem again in the future, but there is nothing preventing the user from setting an encryption policy while the filesystem is still mounted. Fix this by refusing to set an fscrypt context in ext4_set_context() when the block size is larger than the page size. Fixes: 709f0f1 ("ext4: add checks for large folio incompatibilities when BS > PS") Cc: stable@vger.kernel.org # v6.19+ Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Baokun Li <libaokun@linux.alibaba.com> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Encryption is not supported when a filesystem's block size is larger than the page size. ext4_check_large_folio() refuses to mount a filesystem like that if the "encrypt" feature is already enabled, and ext4_set_context() refuses to create encrypted inodes if the feature is enabled after mounting. However, a corrupted or hand-crafted filesystem can still have encrypted inodes even if the "encrypt" feature is not set, and those inodes can be unlocked if they're using v1 policies. Check this in __ext4_iget() and reject them as corrupted. Fixes: 709f0f1 ("ext4: add checks for large folio incompatibilities when BS > PS") Suggested-by: Eric Biggers <ebiggers@kernel.org> Cc: stable@vger.kernel.org # v6.19+ Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Baokun Li <libaokun@linux.alibaba.com> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
blk-crypto-fallback en/decrypts a data unit that fits in one page: the encryption and the decryption path both require the data unit to be contained in a single page-clamped bio_vec. However, the fallback advertises support for all data unit sizes (modes_supported[] is 0xFFFFFFFF), and blk_crypto_start_using_key() never checks the fallback's capabilities, so a key with a larger data unit size is accepted, and fails with BLK_STS_INVAL on the first I/O: EXT4-fs warning (device vdc): ext4_end_bio:360: I/O error 19 writing to inode 15 starting block 70136) EXT4-fs (vdc): failed to convert unwritten extents to written extents -- potential data loss! (inode 15, error -5) where error 19 is BLK_STS_INVAL. Such keys exist on filesystems whose block size is larger than the page size, because fscrypt uses the block size as its data unit size by default. Fix this by advertising only the data unit sizes that the fallback really supports, and by rejecting keys that the fallback can't handle in blk_crypto_fallback_start_using_key(), so that the failure happens when the key is set up instead of during I/O, and before any data is written. Fixes: 488f668 ("block: blk-crypto-fallback for Inline Encryption") Cc: stable@vger.kernel.org Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
__blk_crypto_fallback_decrypt_bio() decrypts the bio one bio_vec at a time: it walks the bio_vecs with __bio_for_each_segment(), requires each bio_vec to be data unit aligned and a multiple of the data unit size, and decrypts every data unit in it with a scatterlist that covers the whole bio_vec. That requires a data unit to be contiguous in memory, and cannot express a data unit that spans more than a page. In preparation for supporting such data units, collect the bio_vecs that make up one data unit in the new blk_crypto_du_bvecs(), and decrypt the data unit with a single crypto request whose scatterlist spans those bio_vecs. Add a struct blk_crypto_du_ctx to describe one data unit, with blk_crypto_du_ctx_init() to initialize it and blk_crypto_du_bvecs() to fill in its bio_vecs. The array that holds them is sized for one bio_vec and lives on the stack, so a data unit that needs more than one bio_vec is rejected with BLK_STS_INVAL for now. Also add a blk_crypto_crypt_du() helper that en/decrypts one data unit. Only the decryption path uses it in this patch; the encryption path will be restructured the same way in a follow-up patch, with the bounce page scatterlist as the destination. No functional change intended. Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
__blk_crypto_fallback_encrypt_bio() encrypts the source bio one bio_vec at a time: it walks the bio with bio_iter_iovec(), which clamps each bio_vec to a single page, encrypts all the data units of that bio_vec into its bounce page with one crypto request per data unit, and starts a new encrypted bio when all of its pages are in use. That structure cannot express a data unit that spans more than a page. In preparation for supporting such data units, restructure the loop around the data unit, the same way as the decryption path: collect the bio_vecs that make up one data unit in blk_crypto_du_bvecs(), encrypt it with a single crypto request whose scatterlist covers exactly those bio_vecs, and advance the source bio by one data unit. Unlike the decryption path, which decrypts in place using one scatterlist for both sides, the encryption path writes the ciphertext to the bounce pages, so it fills the scatterlist for each side of the crypto request, and calls the same blk_crypto_crypt_du() helper, with the encryption direction and the bounce page scatterlist as the destination. A data unit is now encrypted as soon as it is complete instead of when all the data units of its bio_vec are, and the first bio_vec of a data unit is added to the encrypted bio with the whole length of the source bio_vec it starts in, as the data units that follow it in that bio_vec share its bounce page. No functional change intended. Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
blk-crypto-fallback requires each data unit to be contained in a single bio_vec, and fails a bio with a larger data unit with BLK_STS_INVAL. fscrypt uses the filesystem block size as its data unit size, so encrypted files are unusable on filesystems with a block size larger than the page size; the write failure is silent and loses the data. Since commit 5acc6f6 ("fscrypt: Always use blk-crypto for contents on block-based filesystems") there is no fs-layer path left. Fix this by describing a data unit by one bio_vec per page: data units that fit keep using the bio_vec and the scatterlists in the data unit context, and larger ones get arrays from the new blk_crypto_du_ctx_pool. The pool is a mempool backed by a dedicated slab cache, as kmalloc() would round the element size up to the next power of two. It is set up when a key with such a data unit size is first used, and one array is reused for all data units of a bio. A data unit must start at a data unit boundary (a page boundary for a larger one), and the accepted data unit sizes grow to the number of pages that fit in one encrypted bio, rounded down to a power of two. An encrypted bio can run out of room before the source bio ends; free its unused bounce pages with the new blk_crypto_free_enc_pages(). One crypto request per data unit is kept, so the ciphertext of data units that fit in a page is unchanged, and larger data units produce the same ciphertext as inline encryption hardware. Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
A bio that carries an encryption context must cover a whole number of data units: its data unit number is generated from its first data unit (see bio_crypt_set_ctx()), and it is only split at data unit boundaries. iomap attaches the encryption context of an encrypted file to the bios that it builds for direct I/O, but it only cuts them at the logical block size of the device. A bio ends where the bio_vecs that fit into it end, not at a fixed number of bytes: the extraction stops when the bio is out of bio_vecs or when the iterator is exhausted, and a bio_vec can cover a whole folio. A bio can end in the middle of a data unit, and a data unit that is larger than a page makes this reachable without any misaligned input at all: the extraction then stops in the middle of a folio, and a cut at a page granularity inside such a data unit is not data unit aligned even when the position, the length and the buffer are. The fallback rejects such a bio once it reaches the block layer: one of its bio_vecs is not data unit aligned, so a direct I/O that should work fails. By contrast, the equivalent check on the hardware path lives in the splitting code, which a small single segment bio skips, so a bio that starts in the middle of a data unit can reach the hardware without being rejected. Fix this by cutting the bios of an encrypted file at the block size, like the IOMAP_DIO_FSBLOCK_ALIGNED flag does for the filesystems that pass it. fscrypt rejects a data unit size larger than the block size, so a bio that is cut at block size boundaries covers a whole number of data units. This also makes the file systems that don't pass the flag follow the contract, e.g. f2fs, which passes direct I/O that is not aligned to the logical block size of the device on to iomap expecting it to fail with -EINVAL, while iomap checks only the file position and the length, so such a request can reach bio submission. Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Splitting a bio advances the remaining bio's data unit number by the
number of whole data units that were split off:
bio_split()
└─ bio_advance()
└─ bio_crypt_advance()
└─ bio_crypt_dun_increment(dun, bytes >> data_unit_size_bits)
The shift truncates, so splitting inside a data unit leaves the
remainder with the wrong data unit number and silently corrupts data.
The split size is already rounded down to the logical block size, so
this is only possible when the data unit size is larger than it, e.g.
a fscrypt filesystem with a 4K block size on a device with a 512-byte
logical block size.
Fix this by rounding the split size down to a data unit boundary. The
split alignment becomes the maximum of the logical block size, the
data unit size, and the zone write granularity, so a zoned write still
meets all three requirements.
Split bios can be up to one data unit smaller than the queue limits
allow, so a bio can be split a little more often. The number of
segments for the split bio is still counted up to the size before the
rounding, so it can be larger than needed; it is only used for
accounting.
Fixes: a892c8d ("block: Inline encryption support for blk-mq")
Cc: stable@vger.kernel.org
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
bio_split_io_at() fails a bio that it can't split into a valid request with -EINVAL: a bio may have too many discontiguous segments for the queue, may have virtual boundary gaps that leave it without a block sized split, and now that the split size is rounded down to the data unit size of an encryption context, it may have a data unit that does not fit in the room that the queue limits leave for the split. Nothing else says which queue limit caused the failure, and a limit that is smaller than the data unit size, e.g. a max_sectors_kb that was lowered below it, is accepted by the block layer, so warn about it, with the bytes that fit and the alignment involved. Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
bio_split_io_at() rejects an encrypted bio unless every bio_vec is data unit aligned and a multiple of the data unit size, so that every split lands on a data unit boundary. That is stricter than needed: bio_split_alignment() already rounds the split size down to the data unit size, which is all a split needs. The requirement can't be met by all submitters either: a direct I/O read or write adds the pages of the user buffer as they are extracted, and fscrypt_zeroout_range() always adds one page per bio_vec, so a data unit larger than a page is described by bio_vecs that are smaller than it. Drop the requirement: the split size rounding keeps the data unit numbers of the remainder correct. A bio whose first data unit doesn't fit in the split still fails with -EINVAL, as the rounded down size is zero then. The dropped requirement was only ever a proxy for the invariant that the submitter has to keep: the bio must start at a data unit boundary and cover a whole number of data units. Document that in bio_crypt_set_ctx() and in the block API documentation. For safety, warn about a misaligned start in fscrypt_set_bio_crypt_ctx() and reject bios whose size is not a whole number of data units in blk_crypto_submit_bio(). Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
ext4 refuses encryption when the block size is larger than the page size, both at mount time and on a mounted filesystem, and refuses to load an existing encrypted inode. The checks exist because such a block size needs folios that cover a whole block, while file contents were encrypted page by page in the fs-layer path, which cannot handle a folio that covers several data units. Since commit 5acc6f6 ("fscrypt: Always use blk-crypto for contents on block-based filesystems") file contents are always encrypted through blk-crypto, and after the adaptations in the preceding patches, both inline encryption hardware and blk-crypto-fallback support data units larger than a page, so the checks can be removed. Encrypted filesystems still keep their folio order at s_min_folio_order; allowing large folios needs additional work and will be enabled later. Performance: Hardware: 96-core machine, 3.84T NVMe, 512G memory Filesystem: ext4, encryption on the fallback path Buffered sequential 1M writes and reads (1G, fsync) in MiB/s; medians of 3 runs. job 4k plain 4k enc 32k plain 32k enc 4k->32k --------- -------- -------- --------- ------- ------- seqwrite 1981 1044 2062 1399 +34% seqread 1139 931 1791 1793 +93% Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
ext4_mpage_readpages() puts a whole folio into a bio with a single crypt context, taken from the position of the folio, and blk-crypto increments the data unit number for each data unit that follows in the bio. By contrast, fscrypt truncates the data unit numbers to 32 bits for IV_INO_LBLK_32, the only supported IV generation method whose data unit numbers wrap around within a file. The blocks after a wraparound inside a folio are then en/decrypted with IVs of 2^32 + i instead of i. fscrypt_mergeable_bio() only runs between folios, so nothing catches this. This is reachable even without a block size larger than the page size: when a filesystem is mounted without the encrypt feature, ext4_set_max_mapping_order() does not cap its folio order, and enabling the feature on the mounted filesystem does not change that. The encrypted files of a policy that is set up afterwards then get large folios, which can cover the data unit number wraparound of the file. Fix this by sending folios that cover a wraparound to the buffer_head based path in ext4_mpage_readpages(), which creates one bio per block: fscrypt_limit_io_blocks() tells whether the wraparound falls inside the folio's blocks, and only the folio covering it takes the slower path. At most one folio per file is affected: the extra cost is a buffer_head and a bio per block for that folio, and the block layer merges those bios back together. Fixes: 709f0f1 ("ext4: add checks for large folio incompatibilities when BS > PS") Cc: stable@vger.kernel.org # v6.19+ Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Encrypted filesystems keep their folio order at s_min_folio_order, because the read path could not handle a folio that covers a data unit number wraparound. The read path now sends such folios to the buffer_head based path, so drop the restriction and give large folios to all encrypted files, including the ones whose block size is not larger than the page size. Writes and direct I/O don't need the same treatment: ext4_writepages() checks fscrypt_mergeable_bio() for every block, so a write bio never spans a wraparound, and the bios of direct I/O are cut at the block size. Performance: Hardware: 96-core machine, 3.84T NVMe, 512G memory Filesystem: ext4 with a 4k block size, encryption on the fallback path Buffered sequential writes (1G, fsync) in MiB/s; medians of 3 runs. bs Before After gain ---- ------ ------ ---- 4k 1118 1162 +4% 16k 1260 1756 +39% 64k 1272 1977 +55% 256k 1308 2081 +59% 1M 1260 1455 +15% Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Series: https://patchwork.kernel.org/project/linux-fsdevel/list/?series=1183397
Submitter: Baokun Li
Version: 1
Patches: 13/13
Message-ID:
<20261011170958.1759230-1-libaokun@linux.alibaba.com>Base: vfs.base.ci
Lore: https://lore.kernel.org/linux-fsdevel/20261011170958.1759230-1-libaokun@linux.alibaba.com
Automated by ml2pr