Re: [PATCH 2/3] AF_ALG: Drop support for off-CPU cryptography

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch breaks userspace compatibility by forcefully disabling hardware accelerators, asynchronous algorithms, and memory-allocating algorithms, and silently ignores user-provided configuration flags.
--

commit ac5d5083a6e0ac13c785899123f47cf8eacc20ee
Author: Demi Marie Obenour <demiobenour@gmail.com>

AF_ALG: Drop support for off-CPU cryptography

This commit removes support for hardware accelerators in the AF_ALG
subsystem. It forces all userspace crypto API consumers to use purely
software-based synchronous implementations by dropping features and
flags in the bind callbacks. The goal stated is to reduce overhead
and attack surface.

> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 8ccf7a737cd6c..cce000e8590e4 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
[ ... ]
> @@ -181,7 +181,7 @@ static int alg_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int add
>  	if (IS_ERR(type))
>  		return PTR_ERR(type);
>  
> -	private = type->bind(sa->salg_name, sa->salg_feat, sa->salg_mask);
> +	private = type->bind(sa->salg_name);

[Severity: Critical]
Does this silently ignore user-provided configuration flags?

Looking earlier in alg_bind(), the user-supplied salg_feat and salg_mask are
still validated, which explicitly allows the hardware driver flag:

    const u32 allowed = CRYPTO_ALG_KERN_DRIVER_ONLY;
    ...
    if ((sa->salg_feat & ~allowed) || (sa->salg_mask & ~allowed))
        return -EINVAL;

If userspace explicitly requests a hardware driver, it looks like the kernel
will now silently ignore the flag and provide a software fallback instead of
returning an error.

[ ... ]
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 62867daca47d7..7643ba954125a 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
[ ... ]
> @@ -243,4 +243,16 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
>  		    struct af_alg_async_req *areq, size_t maxsize,
>  		    size_t *outlen);
>  
> +/*
> + * Mask used to disable unsupported algorithm implementations.
> + *
> + * This is the same as FSCRYPT_CRYPTOAPI_MASK in fs/crypto/fscrypt_private.h.
> + * In additions to the motivations there, this API is exposed to userspace
> + * that might not be fully trusted.
> + */
> +#define AF_ALG_CRYPTOAPI_MASK                             \
> +	(CRYPTO_ALG_ASYNC | CRYPTO_ALG_ALLOCATES_MEMORY | \
> +	 CRYPTO_ALG_KERN_DRIVER_ONLY)

[Severity: Critical]
Does this unnecessarily break pure software algorithms that legitimately
allocate memory?

While fscrypt restricts memory allocation to prevent filesystem deadlocks,
AF_ALG is a socket API that does not have these same constraints. By
copying the fscrypt mask, this might inadvertently prevent the use of valid
software-only algorithms.

Furthermore, does forcefully removing asynchronous algorithm and hardware
accelerator support break userspace ABI? The kernel generally maintains a
strict no-regression policy for userspace compatibility.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523-af-alg-harden-v1-0-c76755c3a5c5@gmail.com?part=2