[PATCH] bpf: crypto: reject unterminated type and algorithm names

[PATCH] bpf: crypto: reject unterminated type and algorithm names

[Pengpeng Hou]

bpf_crypto_ctx_create() validates the overall size of
struct bpf_crypto_params, but it does not verify that the fixed-width
type[14] and algo[128] fields are NUL-terminated before passing them to
string consumers.

A caller can therefore fill either field without a terminator and cause
bpf_crypto_get_type(), has_algo(), or alloc_tfm() to read past the end
of the fixed buffer.

Reject parameter blocks whose type or algorithm name does not contain a
terminating NUL within the advertised field width.

Fixes: 3e1c6f35409f ("bpf: make common crypto API for TC/XDP programs")
Cc: stable@vger.kernel.org

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 kernel/bpf/crypto.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
index 51f89cecefb4..8732689803a6 100644
--- a/kernel/bpf/crypto.c
+++ b/kernel/bpf/crypto.c
@@ -155,6 +155,12 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
 		return NULL;
 	}
 
+	if (strnlen(params->type, sizeof(params->type)) == sizeof(params->type) ||
+	    strnlen(params->algo, sizeof(params->algo)) == sizeof(params->algo)) {
+		*err = -EINVAL;
+		return NULL;
+	}
+
 	type = bpf_crypto_get_type(params->type);
 	if (IS_ERR(type)) {
 		*err = PTR_ERR(type);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] bpf: crypto: reject unterminated type and algorithm names

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Critical] In-place validation of `struct bpf_crypto_params` is vulnerable to Time-of-Check to Time-of-Use (TOCTOU) race conditions, allowing bypassing of bounds checks and leading to NULL pointer dereferences and out-of-bounds reads.
--

commit 3ccfc91c2a01e60f8c9cc4188d35f3073228f1db
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

bpf: crypto: reject unterminated type and algorithm names

This commit attempts to fix out-of-bounds reads by adding string length checks
for the type and algo fields in bpf_crypto_ctx_create(). It returns -EINVAL if
the strings are not NUL-terminated.

> diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
> --- a/kernel/bpf/crypto.c
> +++ b/kernel/bpf/crypto.c
[ ... ]
> @@ -155,6 +155,12 @@ bpf_crypto_ctx_create(const struct bpf_crypto_params *params, u32 params__sz,
>  		return NULL;
>  	}
>  
> +	if (strnlen(params->type, sizeof(params->type)) == sizeof(params->type) ||
> +	    strnlen(params->algo, sizeof(params->algo)) == sizeof(params->algo)) {

Is there a potential time-of-check to time-of-use race condition here?
If the params pointer refers to a BPF map value, user space or other CPUs
could concurrently modify the map data.

Could the NUL terminator be overwritten immediately after these strnlen()
checks pass, causing downstream consumers like bpf_crypto_get_type() to still
read out-of-bounds?

> +		*err = -EINVAL;
> +		return NULL;
> +	}
> +
>  	type = bpf_crypto_get_type(params->type);

Looking at the rest of bpf_crypto_ctx_create() in the original code, the same
in-place validation pattern appears to affect other fields:

kernel/bpf/crypto.c:bpf_crypto_ctx_create() {
    ...
    if (!!params->authsize ^ !!type->setauthsize) {
        ...
    }
    ...
    if (params->authsize) {
        *err = type->setauthsize(ctx->tfm, params->authsize);
    ...
}

If type->setauthsize is NULL, and params->authsize is initially 0, the XOR
check passes. If params->authsize is concurrently modified to a non-zero
value right after, does the later if (params->authsize) condition evaluate
to true and dereference a NULL pointer?

Similarly, params->key_len is validated against sizeof(params->key):

kernel/bpf/crypto.c:bpf_crypto_ctx_create() {
    ...
    if (!params->key_len || params->key_len > sizeof(params->key)) {
        ...
    }
    ...
    *err = type->setkey(ctx->tfm, params->key, params->key_len);
    ...
}

Could a concurrent modification to params->key_len after the check lead to an
out-of-bounds read in type->setkey()?

Should the entire struct bpf_crypto_params be copied to a local stack buffer
before any validation is performed to prevent these races?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417073128.91029-1-pengpeng@iscas.ac.cn?part=1