[Deepin-Kernel-SIG] [linux 6.6.y] [FROMLIST] [Security] crypto: af_alg - Remove zero-copy support from skcipher and aead#1748
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 support for skcipher and aead by eliminating MSG_SPLICE_PAGES handling and always copying input data into newly allocated pages while keeping the UAPI behavior (e.g., sendfile/splice) functionally intact but safer. Sequence diagram for AF_ALG skcipher/aead sendmsg without zero-copysequenceDiagram
actor User
participant Userspace as Userspace_process
participant AFALG as AF_ALG_socket
participant Sendmsg as af_alg_sendmsg
participant PageAlloc as Page_allocator
User ->> Userspace: sendmsg/sendfile to AF_ALG_socket
Userspace ->> AFALG: sendmsg(fd, msg, size)
AFALG ->> Sendmsg: af_alg_sendmsg(sock, msg, size, ...)
loop for_each_segment
Sendmsg ->> Sendmsg: [ctx.merge]
Sendmsg ->> PageAlloc: alloc_page(GFP_KERNEL)
PageAlloc -->> Sendmsg: struct page *pg
Sendmsg ->> Sendmsg: sg_assign_page(sg[i], pg)
Sendmsg ->> Sendmsg: memcpy_from_msg(page_address(pg), msg, plen)
Sendmsg ->> Sendmsg: sg[i].length = plen
Sendmsg ->> Sendmsg: ctx.used += plen
Sendmsg ->> Sendmsg: ctx.merge = plen & (PAGE_SIZE - 1)
end
Sendmsg -->> AFALG: copied_bytes
AFALG -->> Userspace: return copied_bytes
Userspace -->> User: crypto input queued (no zero-copy)
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
This PR removes AF_ALG zero-copy handling for skcipher/AEAD send paths by forcing incoming data to be copied into kernel-owned pages, reducing TOCTOU and pagecache exposure risks while preserving splice/sendfile compatibility.
Changes:
- Updates AF_ALG sendmsg handling to ignore
MSG_SPLICE_PAGESand always copy input data. - Removes AEAD documentation references to
MSG_SPLICE_PAGES. - Updates userspace AF_ALG documentation to describe copied splice/sendfile behavior.
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 | Replaces zero-copy usage guidance with a compatibility note. |
| crypto/algif_aead.c | Updates the AEAD memory-management comment to remove MSG_SPLICE_PAGES mention. |
| crypto/af_alg.c | Removes the zero-copy sendmsg path and always copies input into allocated pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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. | ||
|
|
|
[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 risky zero-copy support for AF_ALG skcipher and AEAD operations while preserving the existing userspace API semantics.
Bug Fixes:
Enhancements:
Documentation: