[Deepin-Kernel-SIG] [linux 6.18.y] [FROMLIST] [Security] crypto: af_alg - Remove zero-copy support from skcipher and aead#1749
Conversation
The zero-copy support is one of the riskiest aspects of AF_ALG. It allows userspace to request cryptographic operations directly on pagecache pages of files like the 'su' binary. It also allows userspace to concurrently modify the memory which is being operated on, a recipe for TOCTOU vulnerabilities. While zero-copy support is more valuable in other areas of the kernel like the frequently used networking and file I/O code, it has far less value in AF_ALG, which is a niche UAPI. AF_ALG primarily just exists for backwards compatibility with a small set of userspace programs such as 'iwd' that haven't yet been fixed to use userspace crypto code. Originally AF_ALG was intended to be used to access hardware crypto accelerators. However, it isn't an efficient interface for that anyway, and it turned out to be rarely used in this way in practice. Thus, the risks of the zero-copy support in AF_ALG vastly outweigh its benefits. Let's just remove it. This commit removes it from the "skcipher" and "aead" algorithm types. "hash" will be handled separately. This is a soft break, not a hard break. Even after this commit, it still works to use splice() or sendfile() to transfer data to an AF_ALG request socket from a pipe or any file, respectively. What changes is just that the kernel now makes an internal, stable copy of the data before doing the crypto operation. So performance is slightly reduced, but the UAPI isn't broken. And, very importantly, it's much safer. Tested with libkcapi/test.sh. All its test cases still pass. I also verified that this would have prevented the copy.fail exploit as well. I also used a custom test program to verify that sendfile() still works. Fixes: 8ff5909 ("crypto: algif_skcipher - User-space interface for skcipher operations") Fixes: 400c40c ("crypto: algif - add AEAD support") Reported-by: Taeyang Lee <0wn@theori.io> Link: https://copy.fail/ Reported-by: Feng Ning <feng@innora.ai> Closes: https://lore.kernel.org/r/afYcc-tZFwvZZo76@ans-MacBook-Pro.local Reviewed-by: Demi Marie Obenour <demiobenour@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Link: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=ffdd2bc378953b525aca61902534e753f1f8e734 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
Reviewer's GuideRemoves AF_ALG zero‑copy (MSG_SPLICE_PAGES) handling for skcipher/aead so data is always copied into freshly allocated pages before crypto, simplifying af_alg_sendmsg() flow and updating documentation/comments accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull request overview
Removes AF_ALG “zero-copy” input handling for the skcipher and aead algorithm types to reduce TOCTOU-style security risk (e.g., operating directly on pagecache pages), while keeping splice()/sendfile() usable by internally copying data before crypto operations.
Changes:
- Remove MSG_SPLICE_PAGES / extract_iter_to_sg-based TX zero-copy path from
af_alg_sendmsg()and always copy into newly allocated pages. - Update AEAD header comment to remove references to MSG_SPLICE_PAGES.
- Update userspace documentation to describe the removal and the continued splice()/sendfile() support (now via internal copying).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Documentation/crypto/userspace-if.rst | Updates AF_ALG documentation around the former zero-copy interface and splice()/sendfile() behavior |
| crypto/algif_aead.c | Adjusts AEAD userspace API comment to remove MSG_SPLICE_PAGES mention |
| crypto/af_alg.c | Removes TX-side zero-copy page splicing in AF_ALG sendmsg path by always copying into kernel-owned pages |
Comments suppressed due to low confidence (1)
Documentation/crypto/userspace-if.rst:334
- This wording is overly broad: it states AF_ALG zero-copy support was removed entirely, but hash still uses extract_iter_to_sg() (e.g., crypto/algif_hash.c) and thus still has a zero-copy style path. Please narrow the statement to the affected algorithm types (skcipher/aead) or explicitly call out that hash is handled separately.
AF_ALG used to have zero-copy support, but it was removed due to it being a
frequent source of vulnerabilities. For backwards compatibility the splice()
and sendfile() system calls are still supported, but the kernel will make an
internal copy of the data before passing it to the crypto code.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Zero-Copy Interface | ||
| ------------------- |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: opsiff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The zero-copy support is one of the riskiest aspects of AF_ALG. It allows userspace to request cryptographic operations directly on pagecache pages of files like the 'su' binary. It also allows userspace to concurrently modify the memory which is being operated on, a recipe for TOCTOU vulnerabilities.
While zero-copy support is more valuable in other areas of the kernel like the frequently used networking and file I/O code, it has far less value in AF_ALG, which is a niche UAPI. AF_ALG primarily just exists for backwards compatibility with a small set of userspace programs such as 'iwd' that haven't yet been fixed to use userspace crypto code.
Originally AF_ALG was intended to be used to access hardware crypto accelerators. However, it isn't an efficient interface for that anyway, and it turned out to be rarely used in this way in practice.
Thus, the risks of the zero-copy support in AF_ALG vastly outweigh its benefits. Let's just remove it.
This commit removes it from the "skcipher" and "aead" algorithm types. "hash" will be handled separately.
This is a soft break, not a hard break. Even after this commit, it still works to use splice() or sendfile() to transfer data to an AF_ALG request socket from a pipe or any file, respectively. What changes is just that the kernel now makes an internal, stable copy of the data before doing the crypto operation. So performance is slightly reduced, but the UAPI isn't broken. And, very importantly, it's much safer.
Tested with libkcapi/test.sh. All its test cases still pass. I also verified that this would have prevented the copy.fail exploit as well. I also used a custom test program to verify that sendfile() still works.
Fixes: 8ff5909 ("crypto: algif_skcipher - User-space interface for skcipher operations")
Fixes: 400c40c ("crypto: algif - add AEAD support")
Reported-by: Taeyang Lee 0wn@theori.io
Link: https://copy.fail/
Reported-by: Feng Ning feng@innora.ai
Closes: https://lore.kernel.org/r/afYcc-tZFwvZZo76@ans-MacBook-Pro.local
Reviewed-by: Demi Marie Obenour demiobenour@gmail.com
Cc: stable@vger.kernel.org
Link: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=ffdd2bc378953b525aca61902534e753f1f8e734
Summary by Sourcery
Remove zero-copy (MSG_SPLICE_PAGES) support from AF_ALG skcipher and AEAD to harden the userspace crypto interface against TOCTOU-style vulnerabilities.
Bug Fixes:
Enhancements:
Documentation: