summaryrefslogtreecommitdiffstats
path: root/drivers/md/dm-verity-target.c
Commit message (Collapse)AuthorAgeFilesLines
* dm-verity: don't crash if panic_on_corruption is not selectedMikulas Patocka10 days1-3/+6
| | | | | | | | | | | | | | If the user sets panic_on_error and doesn't set panic_on_corruption, dm-verity should not panic on data mismatch. But, currently it panics, because it treats data mismatch as I/O error. This commit fixes the logic so that if there is data mismatch and panic_on_corruption or restart_on_corruption is not selected, the system won't restart or panic. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Fixes: f811b83879fb ("dm-verity: introduce the options restart_on_error and panic_on_error")
* dm-verity: introduce the options restart_on_error and panic_on_errorMikulas Patocka2024-10-021-1/+82
| | | | | | | | | | | | | This patch introduces the options restart_on_error and panic_on_error on dm-verity. Previously, restarting on error was handled by the patch e6a3531dd542cb127c8de32ab1e54a48ae19962b, but Google engineers wanted to have a special option for it. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Suggested-by: Sami Tolvanen <samitolvanen@google.com> Suggested-by: Will Drewry <wad@chromium.org>
* Revert: "dm-verity: restart or panic on an I/O error"Mikulas Patocka2024-10-021-21/+2
| | | | | | | | | | | | | | | | | | | | | | This reverts commit e6a3531dd542cb127c8de32ab1e54a48ae19962b. The problem that the commit e6a3531dd542cb127c8de32ab1e54a48ae19962b fixes was reported as a security bug, but Google engineers working on Android and ChromeOS didn't want to change the default behavior, they want to get -EIO rather than restarting the system, so I am reverting that commit. Note also that calling machine_restart from the I/O handling code is potentially unsafe (the reboot notifiers may wait for the bio that triggered the restart), but Android uses the reboot notifiers to store the reboot reason into the PMU microcontroller, so machine_restart must be used. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org Fixes: e6a3531dd542 ("dm-verity: restart or panic on an I/O error") Suggested-by: Sami Tolvanen <samitolvanen@google.com> Suggested-by: Will Drewry <wad@chromium.org>
* Merge tag 'for-6.12/dm-changes' of ↵Linus Torvalds2024-09-271-2/+21
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm Pull device mapper updates from Mikulas Patocka: - Misc VDO fixes - Remove unused declarations dm_get_rq_mapinfo() and dm_zone_map_bio() - Dm-delay: Improve kernel documentation - Dm-crypt: Allow to specify the integrity key size as an option - Dm-bufio: Remove pointless NULL check - Small code cleanups: Use ERR_CAST; remove unlikely() around IS_ERR; use __assign_bit - Dm-integrity: Fix gcc 5 warning; convert comma to semicolon; fix smatch warning - Dm-integrity: Support recalculation in the 'I' mode - Revert "dm: requeue IO if mapping table not yet available" - Dm-crypt: Small refactoring to make the code more readable - Dm-cache: Remove pointless error check - Dm: Fix spelling errors - Dm-verity: Restart or panic on an I/O error if restart or panic was requested - Dm-verity: Fallback to platform keyring also if key in trusted keyring is rejected * tag 'for-6.12/dm-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm: (26 commits) dm verity: fallback to platform keyring also if key in trusted keyring is rejected dm-verity: restart or panic on an I/O error dm: fix spelling errors dm-cache: remove pointless error check dm vdo: handle unaligned discards correctly dm vdo indexer: Convert comma to semicolon dm-crypt: Use common error handling code in crypt_set_keyring_key() dm-crypt: Use up_read() together with key_put() only once in crypt_set_keyring_key() Revert "dm: requeue IO if mapping table not yet available" dm-integrity: check mac_size against HASH_MAX_DIGESTSIZE in sb_mac() dm-integrity: support recalculation in the 'I' mode dm integrity: Convert comma to semicolon dm integrity: fix gcc 5 warning dm: Make use of __assign_bit() API dm integrity: Remove extra unlikely helper dm: Convert to use ERR_CAST() dm bufio: Remove NULL check of list_entry() dm-crypt: Allow to specify the integrity key size as option dm: Remove unused declaration and empty definition "dm_zone_map_bio" dm delay: enhance kernel documentation ...
| * dm-verity: restart or panic on an I/O errorMikulas Patocka2024-09-261-2/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Maxim Suhanov reported that dm-verity doesn't crash if an I/O error happens. In theory, this could be used to subvert security, because an attacker can create sectors that return error with the Write Uncorrectable command. Some programs may misbehave if they have to deal with EIO. This commit fixes dm-verity, so that if "panic_on_corruption" or "restart_on_corruption" was specified and an I/O error happens, the machine will panic or restart. This commit also changes kernel_restart to emergency_restart - kernel_restart calls reboot notifiers and these reboot notifiers may wait for the bio that failed. emergency_restart doesn't call the notifiers. Reported-by: Maxim Suhanov <dfirblog@gmail.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org
* | dm-verity: expose root hash digest and signature data to LSMsDeven Bowers2024-08-201-0/+118
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | dm-verity provides a strong guarantee of a block device's integrity. As a generic way to check the integrity of a block device, it provides those integrity guarantees to its higher layers, including the filesystem level. However, critical security metadata like the dm-verity roothash and its signing information are not easily accessible to the LSMs. To address this limitation, this patch introduces a mechanism to store and manage these essential security details within a newly added LSM blob in the block_device structure. This addition allows LSMs to make access control decisions on the integrity data stored within the block_device, enabling more flexible security policies. For instance, LSMs can now revoke access to dm-verity devices based on their roothashes, ensuring that only authorized and verified content is accessible. Additionally, LSMs can enforce policies to only allow files from dm-verity devices that have a valid digital signature to execute, effectively blocking any unsigned files from execution, thus enhancing security against unauthorized modifications. The patch includes new hook calls, `security_bdev_setintegrity()`, in dm-verity to expose the dm-verity roothash and the roothash signature to LSMs via preresume() callback. By using the preresume() callback, it ensures that the security metadata is consistently in sync with the metadata of the dm-verity target in the current active mapping table. The hook calls are depended on CONFIG_SECURITY. Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com> Signed-off-by: Fan Wu <wufan@linux.microsoft.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> [PM: moved sig_size field as discussed] Signed-off-by: Paul Moore <paul@paul-moore.com>
* dm-verity: fix dm_is_verity_target() when dm-verity is builtinEric Biggers2024-07-101-8/+8
| | | | | | | | | | | | | When CONFIG_DM_VERITY=y, dm_is_verity_target() returned true for any builtin dm target, not just dm-verity. Fix this by checking for verity_target instead of THIS_MODULE (which is NULL for builtin code). Fixes: b6c1c5745ccc ("dm: Add verity helpers for LoadPin") Cc: stable@vger.kernel.org Cc: Matthias Kaehlcke <mka@chromium.org> Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm: stop using blk_limits_io_{min,opt}Christoph Hellwig2024-07-101-1/+1
| | | | | | | | | | | | Remove use of the blk_limits_io_{min,opt} and assign the values directly to the queue_limits structure. For the io_opt this is a completely mechanical change, for io_min it removes flooring the limit to the physical and logical block size in the particular caller. But as blk_validate_limits will do the same later when actually applying the limits, there still is no change in overall behavior. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: hash blocks with shash import+finup when possibleEric Biggers2024-07-031-50/+119
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently dm-verity computes the hash of each block by using multiple calls to the "ahash" crypto API. While the exact sequence depends on the chosen dm-verity settings, in the vast majority of cases it is: 1. crypto_ahash_init() 2. crypto_ahash_update() [salt] 3. crypto_ahash_update() [data] 4. crypto_ahash_final() This is inefficient for two main reasons: - It makes multiple indirect calls, which is expensive on modern CPUs especially when mitigations for CPU vulnerabilities are enabled. Since the salt is the same across all blocks on a given dm-verity device, a much more efficient sequence would be to do an import of the pre-salted state, then a finup. - It uses the ahash (asynchronous hash) API, despite the fact that CPU-based hashing is almost always used in practice, and therefore it experiences the overhead of the ahash-based wrapper for shash. Because dm-verity was intentionally converted to ahash to support off-CPU crypto accelerators, a full reversion to shash might not be acceptable. Yet, we should still provide a fast path for shash with the most common dm-verity settings. Another reason for shash over ahash is that the upcoming multibuffer hashing support, which is specific to CPU-based hashing, is much better suited for shash than for ahash. Supporting it via ahash would add significant complexity and overhead. And it's not possible for the "same" code to properly support both multibuffer hashing and HW accelerators at the same time anyway, given the different computation models. Unfortunately there will always be code specific to each model needed (for users who want to support both). Therefore, this patch adds a new shash import+finup based fast path to dm-verity. It is used automatically when appropriate. This makes dm-verity optimized for what the vast majority of users want: CPU-based hashing with the most common settings, while still retaining support for rarer settings and off-CPU crypto accelerators. In benchmarks with veritysetup's default parameters (SHA-256, 4K data and hash block sizes, 32-byte salt), which also match the parameters that Android currently uses, this patch improves block hashing performance by about 15% on x86_64 using the SHA-NI instructions, or by about 5% on arm64 using the ARMv8 SHA2 instructions. On x86_64 roughly two-thirds of the improvement comes from the use of import and finup, while the remaining third comes from the switch from ahash to shash. Note that another benefit of using "import" to handle the salt is that if the salt size is equal to the input size of the hash algorithm's compression function, e.g. 64 bytes for SHA-256, then the performance is exactly the same as no salt. This doesn't seem to be much better than veritysetup's current default of 32-byte salts, due to the way SHA-256's finalization padding works, but it should be marginally better. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: make verity_hash() take dm_verity_io instead of ahash_requestEric Biggers2024-07-031-11/+10
| | | | | | | | | | | In preparation for adding shash support to dm-verity, change verity_hash() to take a pointer to a struct dm_verity_io instead of a pointer to the ahash_request embedded inside it. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: always "map" the data blocksEric Biggers2024-07-031-145/+37
| | | | | | | | | | | | | | | dm-verity needs to access data blocks by virtual address in three different cases (zeroization, recheck, and forward error correction), and one more case (shash support) is coming. Since it's guaranteed that dm-verity data blocks never cross pages, and kmap_local_page and kunmap_local are no-ops on modern platforms anyway, just unconditionally "map" every data block's page and work with the virtual buffer directly. This simplifies the code and eliminates unnecessary overhead. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: provide dma_alignment limit in io_hintsEric Biggers2024-07-031-0/+8
| | | | | | | | | | | | | | | | | | | | | | | Since Linux v6.1, some filesystems support submitting direct I/O that is aligned to only dma_alignment instead of the logical_block_size alignment that was required before. I/O that is not aligned to the logical_block_size is difficult to handle in device-mapper targets that do cryptographic processing of data, as it makes the units of data that are hashed or encrypted possibly be split across pages, creating rarely used and rarely tested edge cases. As such, dm-crypt and dm-integrity have already opted out of this by setting dma_alignment to 'logical_block_size - 1'. Although dm-verity does have code that handles these cases (or at least is intended to do so), supporting direct I/O with such a low amount of alignment is not really useful on dm-verity devices. So, opt dm-verity out of it too so that it's not necessary to handle these edge cases. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: make real_digest and want_digest fixed-lengthEric Biggers2024-07-031-2/+1
| | | | | | | | | | | | | | | | | | | Change the digest fields in struct dm_verity_io from variable-length to fixed-length, since their maximum length is fixed at HASH_MAX_DIGESTSIZE, i.e. 64 bytes, which is not too big. This is simpler and makes the fields a bit faster to access. (HASH_MAX_DIGESTSIZE did not exist when this code was written, which may explain why it wasn't used.) This makes the verity_io_real_digest() and verity_io_want_digest() functions trivial, but this patch leaves them in place temporarily since most of their callers will go away in a later patch anyway. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: move data hash mismatch handling into its own functionEric Biggers2024-07-031-28/+36
| | | | | | | | | | Move the code that handles mismatches of data block hashes into its own function so that it doesn't clutter up verity_verify_io(). Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* dm-verity: move hash algorithm setup into its own functionEric Biggers2024-07-021-31/+39
| | | | | | | | | | Move the code that sets up the hash transformation into its own function. No change in behavior. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
* Merge tag 'for-6.9/dm-bh-wq' of ↵Linus Torvalds2024-03-131-22/+42
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm Pull device mapper BH workqueue conversion from Mike Snitzer: "Convert the DM verity and crypt targets from (ab)using tasklets to using BH workqueues. These changes were coordinated with Tejun and are based ontop of DM's 6.9 changes and Tejun's 6.9 workqueue tree" * tag 'for-6.9/dm-bh-wq' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm: dm-verity: Convert from tasklet to BH workqueue dm-crypt: Convert from tasklet to BH workqueue
| * dm-verity: Convert from tasklet to BH workqueueTejun Heo2024-03-021-22/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The only generic interface to execute asynchronously in the BH context is tasklet; however, it's marked deprecated and has some design flaws. To replace tasklets, BH workqueue support was recently added. A BH workqueue behaves similarly to regular workqueues except that the queued work items are executed in the BH context. This commit converts dm-verity from tasklet to BH workqueue. It backfills tasklet code that was removed with commit 0a9bab391e33 ("dm-crypt, dm-verity: disable tasklets") and tweaks to use BH workqueue (and does some renaming). This is a minimal conversion which doesn't rename the related names including the "try_verify_in_tasklet" option. If this patch is applied, a follow-up patch would be necessary. I couldn't decide whether the option name would need to be updated too. Signed-off-by: Tejun Heo <tj@kernel.org> [snitzer: rename 'use_tasklet' to 'use_bh_wq' and 'in_tasklet' to 'in_bh'] Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* | Merge tag 'for-6.9/dm-changes' of ↵Linus Torvalds2024-03-131-8/+15
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm Pull device mapper updates from Mike Snitzer: - Fix DM core's IO submission (which include dm-io and dm-bufio) such that a bio's IO priority is propagated. Work focused on enabling both DM crypt and verity targets to retain the appropriate IO priority - Fix DM raid reshape logic to not allow an empty flush bio to be requeued due to false concern about the bio, which doesn't have a data payload, accessing beyond the end of the device - Fix DM core's internal resume so that it properly calls both presume and resume methods, which fixes the potential for a postsuspend and resume imbalance - Update DM verity target to set DM_TARGET_SINGLETON flag because it doesn't make sense to have a DM table with a mix of targets that include dm-verity - Small cleanups in DM crypt, thin, and integrity targets - Fix references to dm-devel mailing list to use latest list address * tag 'for-6.9/dm-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm: dm: call the resume method on internal suspend dm raid: fix false positive for requeue needed during reshape dm-integrity: set max_integrity_segments in dm_integrity_io_hints dm: update relevant MODULE_AUTHOR entries to latest dm-devel mailing list dm ioctl: update DM_DRIVER_EMAIL to new dm-devel mailing list dm verity: set DM_TARGET_SINGLETON feature flag dm crypt: Fix IO priority lost when queuing write bios dm verity: Fix IO priority lost when reading FEC and hash dm bufio: Support IO priority dm io: Support IO priority dm crypt: remove redundant state settings after waking up dm thin: add braces around conditional code that spans lines
| * dm verity: set DM_TARGET_SINGLETON feature flagFan Wu2024-02-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | The device-mapper has a flag to mark targets as singleton, which is a required flag for immutable targets. Without this flag, multiple dm-verity targets can be added to a mapped device, which has no practical use cases and will let dm_table_get_immutable_target return NULL. This patch adds the missing flag, restricting only one dm-verity target per mapped device. Signed-off-by: Fan Wu <wufan@linux.microsoft.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm verity: Fix IO priority lost when reading FEC and hashHongyu Jin2024-02-201-6/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After obtaining the data, verification or error correction process may trigger a new IO that loses the priority of the original IO, that is, the verification of the higher priority IO may be blocked by the lower priority IO. Make the IO used for verification and error correction follow the priority of the original IO. Co-developed-by: Yibin Ding <yibin.ding@unisoc.com> Signed-off-by: Yibin Ding <yibin.ding@unisoc.com> Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com> Reviewed-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm io: Support IO priorityHongyu Jin2024-02-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some IO will dispatch from kworker with different io_context settings than the submitting task, we may need to specify a priority to avoid losing priority. Add IO priority parameter to dm_io() and update all callers. Co-developed-by: Yibin Ding <yibin.ding@unisoc.com> Signed-off-by: Yibin Ding <yibin.ding@unisoc.com> Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com> Reviewed-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* | dm-integrity, dm-verity: reduce stack usage for recheckArnd Bergmann2024-02-241-2/+2
|/ | | | | | | | | | | | | | | | | | | | | | | | | | The newly added integrity_recheck() function has another larger stack allocation, just like its caller integrity_metadata(). When it gets inlined, the combination of the two exceeds the warning limit for 32-bit architectures and possibly risks an overflow when this is called from a deep call chain through a file system: drivers/md/dm-integrity.c:1767:13: error: stack frame size (1048) exceeds limit (1024) in 'integrity_metadata' [-Werror,-Wframe-larger-than] 1767 | static void integrity_metadata(struct work_struct *w) Since the caller at this point is done using its checksum buffer, just reuse the same buffer in the new function to avoid the double allocation. [Mikulas: add "noinline" to integrity_recheck and verity_recheck. These functions are only called on error, so they shouldn't bloat the stack frame or code size of the caller.] Fixes: c88f5e553fe3 ("dm-integrity: recheck the integrity tag after a failure") Fixes: 9177f3c0dea6 ("dm-verity: recheck the hash after a failure") Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm-crypt, dm-integrity, dm-verity: bump target versionMike Snitzer2024-02-201-1/+1
| | | | Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm-verity: recheck the hash after a failureMikulas Patocka2024-02-201-6/+80
| | | | | | | | | | | | | | | | | If a userspace process reads (with O_DIRECT) multiple blocks into the same buffer, dm-verity reports an error [1]. This commit fixes dm-verity, so that if hash verification fails, the data is read again into a kernel buffer (where userspace can't modify it) and the hash is rechecked. If the recheck succeeds, the content of the kernel buffer is copied into the user buffer; if the recheck fails, an error is reported. [1] https://people.redhat.com/~mpatocka/testcases/blk-auth-modify/read2.c Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm-crypt, dm-verity: disable taskletsMikulas Patocka2024-02-021-24/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Tasklets have an inherent problem with memory corruption. The function tasklet_action_common calls tasklet_trylock, then it calls the tasklet callback and then it calls tasklet_unlock. If the tasklet callback frees the structure that contains the tasklet or if it calls some code that may free it, tasklet_unlock will write into free memory. The commits 8e14f610159d and d9a02e016aaf try to fix it for dm-crypt, but it is not a sufficient fix and the data corruption can still happen [1]. There is no fix for dm-verity and dm-verity will write into free memory with every tasklet-processed bio. There will be atomic workqueues implemented in the kernel 6.9 [2]. They will have better interface and they will not suffer from the memory corruption problem. But we need something that stops the memory corruption now and that can be backported to the stable kernels. So, I'm proposing this commit that disables tasklets in both dm-crypt and dm-verity. This commit doesn't remove the tasklet support, because the tasklet code will be reused when atomic workqueues will be implemented. [1] https://lore.kernel.org/all/d390d7ee-f142-44d3-822a-87949e14608b@suse.de/T/ [2] https://lore.kernel.org/lkml/20240130091300.2968534-1-tj@kernel.org/ Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org Fixes: 39d42fa96ba1b ("dm crypt: add flags to optionally bypass kcryptd workqueues") Fixes: 5721d4e5a9cdb ("dm verity: Add optional "try_verify_in_tasklet" feature") Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: don't perform FEC for failed readahead IOWu Bo2023-11-291-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We found an issue under Android OTA scenario that many BIOs have to do FEC where the data under dm-verity is 100% complete and no corruption. Android OTA has many dm-block layers, from upper to lower: dm-verity dm-snapshot dm-origin & dm-cow dm-linear ufs DM tables have to change 2 times during Android OTA merging process. When doing table change, the dm-snapshot will be suspended for a while. During this interval, many readahead IOs are submitted to dm_verity from filesystem. Then the kverity works are busy doing FEC process which cost too much time to finish dm-verity IO. This causes needless delay which feels like system is hung. After adding debugging it was found that each readahead IO needed around 10s to finish when this situation occurred. This is due to IO amplification: dm-snapshot suspend erofs_readahead // 300+ io is submitted dm_submit_bio (dm_verity) dm_submit_bio (dm_snapshot) bio return EIO bio got nothing, it's empty verity_end_io verity_verify_io forloop range(0, io->n_blocks) // each io->nblocks ~= 20 verity_fec_decode fec_decode_rsb fec_read_bufs forloop range(0, v->fec->rsn) // v->fec->rsn = 253 new_read submit_bio (dm_snapshot) end loop end loop dm-snapshot resume Readahead BIOs get nothing while dm-snapshot is suspended, so all of them will cause verity's FEC. Each readahead BIO needs to verify ~20 (io->nblocks) blocks. Each block needs to do FEC, and every block needs to do 253 (v->fec->rsn) reads. So during the suspend interval(~200ms), 300 readahead BIOs trigger ~1518000 (300*20*253) IOs to dm-snapshot. As readahead IO is not required by userspace, and to fix this issue, it is best to pass readahead errors to upper layer to handle it. Cc: stable@vger.kernel.org Fixes: a739ff3f543a ("dm verity: add support for forward error correction") Signed-off-by: Wu Bo <bo.wu@vivo.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: initialize fec io before freeing itWu Bo2023-11-291-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | If BIO error, verity_end_io() can call verity_finish_io() before verity_fec_init_io(). Therefore, fec_io->rs is not initialized and may crash when doing memory freeing in verity_fec_finish_io(). Crash call stack: die+0x90/0x2b8 __do_kernel_fault+0x260/0x298 do_bad_area+0x2c/0xdc do_translation_fault+0x3c/0x54 do_mem_abort+0x54/0x118 el1_abort+0x38/0x5c el1h_64_sync_handler+0x50/0x90 el1h_64_sync+0x64/0x6c free_rs+0x18/0xac fec_rs_free+0x10/0x24 mempool_free+0x58/0x148 verity_fec_finish_io+0x4c/0xb0 verity_end_io+0xb8/0x150 Cc: stable@vger.kernel.org # v6.0+ Fixes: 5721d4e5a9cd ("dm verity: Add optional "try_verify_in_tasklet" feature") Signed-off-by: Wu Bo <bo.wu@vivo.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm-verity: don't use blocking calls from taskletsMikulas Patocka2023-11-171-11/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The commit 5721d4e5a9cd enhanced dm-verity, so that it can verify blocks from tasklets rather than from workqueues. This reportedly improves performance significantly. However, dm-verity was using the flag CRYPTO_TFM_REQ_MAY_SLEEP from tasklets which resulted in warnings about sleeping function being called from non-sleeping context. BUG: sleeping function called from invalid context at crypto/internal.h:206 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 14, name: ksoftirqd/0 preempt_count: 100, expected: 0 RCU nest depth: 0, expected: 0 CPU: 0 PID: 14 Comm: ksoftirqd/0 Tainted: G W 6.7.0-rc1 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x32/0x50 __might_resched+0x110/0x160 crypto_hash_walk_done+0x54/0xb0 shash_ahash_update+0x51/0x60 verity_hash_update.isra.0+0x4a/0x130 [dm_verity] verity_verify_io+0x165/0x550 [dm_verity] ? free_unref_page+0xdf/0x170 ? psi_group_change+0x113/0x390 verity_tasklet+0xd/0x70 [dm_verity] tasklet_action_common.isra.0+0xb3/0xc0 __do_softirq+0xaf/0x1ec ? smpboot_thread_fn+0x1d/0x200 ? sort_range+0x20/0x20 run_ksoftirqd+0x15/0x30 smpboot_thread_fn+0xed/0x200 kthread+0xdc/0x110 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x28/0x40 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork_asm+0x11/0x20 </TASK> This commit fixes dm-verity so that it doesn't use the flags CRYPTO_TFM_REQ_MAY_SLEEP and CRYPTO_TFM_REQ_MAY_BACKLOG from tasklets. The crypto API would do GFP_ATOMIC allocation instead, it could return -ENOMEM and we catch -ENOMEM in verity_tasklet and requeue the request to the workqueue. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org # v6.0+ Fixes: 5721d4e5a9cd ("dm verity: Add optional "try_verify_in_tasklet" feature") Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* block: replace fmode_t with a block-specific type for block open flagsChristoph Hellwig2023-06-121-3/+3
| | | | | | | | | | | | | | The only overlap between the block open flags mapped into the fmode_t and other uses of fmode_t are FMODE_READ and FMODE_WRITE. Define a new blk_mode_t instead for use in blkdev_get_by_{dev,path}, ->open and ->ioctl and stop abusing fmode_t. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Jack Wang <jinpu.wang@ionos.com> [rnbd] Reviewed-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Christian Brauner <brauner@kernel.org> Link: https://lore.kernel.org/r/20230608110258.189493-28-hch@lst.de Signed-off-by: Jens Axboe <axboe@kernel.dk>
* dm: add helper macro for simple DM target module init and exitYangtao Li2023-04-111-13/+1
| | | | | | | | | | | | Eliminate duplicate boilerplate code for simple modules that contain a single DM target driver without any additional setup code. Add a new module_dm() macro, which replaces the module_init() and module_exit() with template functions that call dm_register_target() and dm_unregister_target() respectively. Signed-off-by: Yangtao Li <frank.li@vivo.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: push error reporting down to dm_register_target()Yangtao Li2023-04-111-7/+1
| | | | | | | | Simplifies each DM target's init method by making dm_register_target() responsible for its error reporting (on behalf of targets). Signed-off-by: Yangtao Li <frank.li@vivo.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: emit audit events on verification failure and moreMichael Weiß2023-04-041-2/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | dm-verity signals integrity violations by returning I/O errors to user space. To identify integrity violations by a controlling instance, the kernel audit subsystem can be used to emit audit events to user space. Analogous to dm-integrity, we also use the dm-audit submodule allowing to emit audit events on verification failures of metadata and data blocks as well as if max corrupted errors are reached. The construction and destruction of verity device mappings are also relevant for auditing a system. Thus, those events are also logged as audit events. Tested by starting a container with the container manager (cmld) of GyroidOS which uses a dm-verity protected rootfs image root.img mapped to /dev/mapper/<uuid>-root. One block was manipulated in the underlying image file and repeated reads of the verity device were performed again until the max corrupted errors is reached, e.g.: dd if=/dev/urandom of=root.img bs=512 count=1 seek=1000 for i in range {1..101}; do \ dd if=/dev/mapper/<uuid>-root of=/dev/null bs=4096 \ count=1 skip=1000 \ done The resulting audit log looks as follows: type=DM_CTRL msg=audit(1677618791.876:962): module=verity op=ctr ppid=4876 pid=29102 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=44 comm="cmld" exe="/usr/sbin/cml/cmld" subj=unconfined dev=254:3 error_msg='success' res=1 type=DM_EVENT msg=audit(1677619463.786:1074): module=verity op=verify-data dev=7:0 sector=1000 res=0 ... type=DM_EVENT msg=audit(1677619596.727:1162): module=verity op=verify-data dev=7:0 sector=1000 res=0 type=DM_EVENT msg=audit(1677619596.731:1163): module=verity op=max-corrupted-errors dev=254:3 sector=? res=0 Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de> Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: fix error handling for check_at_most_once on FECYeongjin Gil2023-04-041-1/+1
| | | | | | | | | | | | | | | | | | | | | | In verity_end_io(), if bi_status is not BLK_STS_OK, it can be return directly. But if FEC configured, it is desired to correct the data page through verity_verify_io. And the return value will be converted to blk_status and passed to verity_finish_io(). BTW, when a bit is set in v->validated_blocks, verity_verify_io() skips verification regardless of I/O error for the corresponding bio. In this case, the I/O error could not be returned properly, and as a result, there is a problem that abnormal data could be read for the corresponding block. To fix this problem, when an I/O error occurs, do not skip verification even if the bit related is set in v->validated_blocks. Fixes: 843f38d382b1 ("dm verity: add 'check_at_most_once' option to only validate hashes once") Cc: stable@vger.kernel.org Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com> Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: avoid useless 'else' after 'break' or return'Heinz Mauelshagen2023-02-141-16/+17
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: avoid using symbolic permissionsHeinz Mauelshagen2023-02-141-1/+1
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: prefer '"%s...", __func__'Heinz Mauelshagen2023-02-141-2/+2
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: add missing empty linesHeinz Mauelshagen2023-02-141-0/+4
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: fix trailing statementsHeinz Mauelshagen2023-02-141-4/+2
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: change "unsigned" to "unsigned int"Heinz Mauelshagen2023-02-141-15/+15
| | | | | Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: stop using WQ_UNBOUND for verify_wqNathan Huckleberry2023-02-021-5/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Setting WQ_UNBOUND increases scheduler latency on ARM64. This is likely due to the asymmetric architecture of ARM64 processors. I've been unable to reproduce the results that claim WQ_UNBOUND gives a performance boost on x86-64. This flag is causing performance issues for multiple subsystems within Android. Notably, the same slowdown exists for decompression with EROFS. | open-prebuilt-camera | WQ_UNBOUND | ~WQ_UNBOUND | |-----------------------|------------|---------------| | verity wait time (us) | 11746 | 119 (-98%) | | erofs wait time (us) | 357805 | 174205 (-51%) | | sha256 ramdisk random read | WQ_UNBOUND | ~WQ_UNBOUND | |----------------------------|-----------=---|-------------| | arm64 (accelerated) | bw=42.4MiB/s | bw=212MiB/s | | arm64 (generic) | bw=16.5MiB/s | bw=48MiB/s | | x86_64 (generic) | bw=233MiB/s | bw=230MiB/s | Using a alloc_workqueue() @max_active arg of num_online_cpus() only made sense with WQ_UNBOUND. Switch the @max_active arg to 0 (aka default, which is 256 per-cpu). Also, eliminate 'wq_flags' since it really doesn't serve a purpose. Cc: Sami Tolvanen <samitolvanen@google.com> Cc: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Nathan Huckleberry <nhuck@google.com> Reviewed-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm verity: enable WQ_HIGHPRI on verify_wqNathan Huckleberry2022-10-181-8/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | WQ_HIGHPRI increases throughput and decreases disk latency when using dm-verity. This is important in Android for camera startup speed. The following tests were run by doing 60 seconds of random reads using a dm-verity device backed by two ramdisks. Without WQ_HIGHPRI lat (usec): min=13, max=3947, avg=69.53, stdev=50.55 READ: bw=51.1MiB/s (53.6MB/s), 51.1MiB/s-51.1MiB/s (53.6MB/s-53.6MB/s) With WQ_HIGHPRI: lat (usec): min=13, max=7854, avg=31.15, stdev=30.42 READ: bw=116MiB/s (121MB/s), 116MiB/s-116MiB/s (121MB/s-121MB/s) Further testing was done by measuring how long it takes to open a camera on an Android device. Without WQ_HIGHPRI Total verity work queue wait times (ms): 880.960, 789.517, 898.852 With WQ_HIGHPRI: Total verity work queue wait times (ms): 528.824, 439.191, 433.300 The average time to open the camera is reduced by 350ms (or 40-50%). Signed-off-by: Nathan Huckleberry <nhuck@google.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* dm: verity-loadpin: Only trust verity targets with enforcementMatthias Kaehlcke2022-09-071-0/+16
| | | | | | | | | | | | | | | | Verity targets can be configured to ignore corrupted data blocks. LoadPin must only trust verity targets that are configured to perform some kind of enforcement when data corruption is detected, like returning an error, restarting the system or triggering a panic. Fixes: b6c1c5745ccc ("dm: Add verity helpers for LoadPin") Reported-by: Sarthak Kukreti <sarthakkukreti@chromium.org> Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Reviewed-by: Sarthak Kukreti <sarthakkukreti@chromium.org> Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20220907133055.1.Ic8a1dafe960dc0f8302e189642bc88ebb785d274@changeid
* Merge tag 'for-6.0/dm-fixes' of ↵Linus Torvalds2022-08-111-2/+12
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm Pull device mapper fixes from Mike Snitzer: - A few fixes for the DM verity and bufio changes in this merge window - A smatch warning fix for DM writecache locking in writecache_map * tag 'for-6.0/dm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm: dm bufio: fix some cases where the code sleeps with spinlock held dm writecache: fix smatch warning about invalid return from writecache_map dm verity: fix verity_parse_opt_args parsing dm verity: fix DM_VERITY_OPTS_MAX value yet again dm bufio: simplify DM_BUFIO_CLIENT_NO_SLEEP locking
| * dm verity: fix verity_parse_opt_args parsingMike Snitzer2022-08-091-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit df326e7a0699 ("dm verity: allow optional args to alter primary args handling") introduced a bug where verity_parse_opt_args() wouldn't properly shift past an optional argument's additional params (by ignoring them). Fix this by avoiding returning with error if an unknown argument is encountered when @only_modifier_opts=true is passed to verity_parse_opt_args(). In practice this regressed the cryptsetup testsuite's FEC testing because unknown optional arguments were encountered, wherey short-circuiting ever testing FEC mode. With this fix all of the cryptsetup testsuite's verity FEC tests pass. Fixes: df326e7a0699 ("dm verity: allow optional args to alter primary args handling") Reported-by: Milan Broz <gmazyland@gmail.com>> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm verity: fix DM_VERITY_OPTS_MAX value yet againMike Snitzer2022-08-091-1/+1
| | | | | | | | | | | | | | | | | | | | | | Must account for the possibility that "try_verify_in_tasklet" is used. This is the same issue that was fixed with commit 160f99db94322 -- it is far too easy to miss that additional a new argument(s) require bumping DM_VERITY_OPTS_MAX accordingly. Fixes: 5721d4e5a9cd ("dm verity: Add optional "try_verify_in_tasklet" feature") Signed-off-by: Mike Snitzer <snitzer@kernel.org>
* | Merge tag 'for-6.0/dm-changes-2' of ↵Linus Torvalds2022-08-061-21/+139
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm Pull more device mapper updates from Mike Snitzer: - Add flags argument to dm_bufio_client_create and introduce DM_BUFIO_CLIENT_NO_SLEEP flag to have dm-bufio use spinlock rather than mutex for its locking. - Add optional "try_verify_in_tasklet" feature to DM verity target. This feature gives users the option to improve IO latency by using a tasklet to verify, using hashes in bufio's cache, rather than wait to schedule a work item via workqueue. But if there is a bufio cache miss, or an error, then the tasklet will fallback to using workqueue. - Incremental changes to both dm-bufio and the DM verity target to use jump_label to minimize cost of branching associated with the niche "try_verify_in_tasklet" feature. DM-bufio in particular is used by quite a few other DM targets so it doesn't make sense to incur additional bufio cost in those targets purely for the benefit of this niche verity feature if the feature isn't ever used. - Optimize verity_verify_io, which is used by both workqueue and tasklet based verification, if FEC is not configured or tasklet based verification isn't used. - Remove DM verity target's verify_wq's use of the WQ_CPU_INTENSIVE flag since it uses WQ_UNBOUND. Also, use the WQ_HIGHPRI flag if "try_verify_in_tasklet" is specified. * tag 'for-6.0/dm-changes-2' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm: dm verity: have verify_wq use WQ_HIGHPRI if "try_verify_in_tasklet" dm verity: remove WQ_CPU_INTENSIVE flag since using WQ_UNBOUND dm verity: only copy bvec_iter in verity_verify_io if in_tasklet dm verity: optimize verity_verify_io if FEC not configured dm verity: conditionally enable branching for "try_verify_in_tasklet" dm bufio: conditionally enable branching for DM_BUFIO_CLIENT_NO_SLEEP dm verity: allow optional args to alter primary args handling dm verity: Add optional "try_verify_in_tasklet" feature dm bufio: Add DM_BUFIO_CLIENT_NO_SLEEP flag dm bufio: Add flags argument to dm_bufio_client_create
| * dm verity: have verify_wq use WQ_HIGHPRI if "try_verify_in_tasklet"Mike Snitzer2022-08-041-1/+11
| | | | | | | | | | | | | | | | | | Allow verify_wq to preempt softirq since verification in tasklet will fall-back to using it for error handling (or if the bufio cache doesn't have required hashes). Suggested-by: Nathan Huckleberry <nhuck@google.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm verity: remove WQ_CPU_INTENSIVE flag since using WQ_UNBOUNDMike Snitzer2022-08-041-1/+1
| | | | | | | | | | | | | | | | | | | | The documentation [1] says that WQ_CPU_INTENSIVE is "meaningless" for unbound wq. So remove WQ_CPU_INTENSIVE from the verify_wq allocation. 1. https://www.kernel.org/doc/html/latest/core-api/workqueue.html#flags Suggested-by: Maksym Planeta <mplaneta@os.inf.tu-dresden.de> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm verity: only copy bvec_iter in verity_verify_io if in_taskletMike Snitzer2022-08-041-9/+16
| | | | | | | | | | | | | | Avoid extra bvec_iter copy unless it is needed to allow retrying verification, that failed from a tasklet, from a workqueue. Signed-off-by: Mike Snitzer <snitzer@kernel.org>
| * dm verity: optimize verity_verify_io if FEC not configuredMike Snitzer2022-08-041-1/+8
| | | | | | | | | | | | | | Only declare and copy bvec_iter if CONFIG_DM_VERITY_FEC is defined and FEC enabled for the verity device. Signed-off-by: Mike Snitzer <snitzer@kernel.org>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-14 10:54:15 +0000