Re: [PATCH v5 6/8] s390/crypto: Fix handling of EBUSY in PHMAC when req is pushed to crypto engine

On 2026-08-20 16:35, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incorrect module teardown order in `s390_phmac_exit` leads to
> kthread leaks and Use-After-Free of the `crypto_engine`.
> --
> 
> --- Patch [6]: [PATCH v5 6/8] s390/crypto: Fix handling of EBUSY in
> PHMAC when req is pushed to crypto engine ---
> commit c74c807c2aeebffc7e981ca45b5b377727342ad1
> Author: Harald Freudenberger <[email protected]>
> 
> s390/crypto: Fix handling of EBUSY in PHMAC when req is pushed to 
> crypto engine
> 
> When a request is transferred to the engine via
> crypto_transfer_hash_request_to_engine() there are two return codes
> signaling a successful transfer: EINPROGRESS and EBUSY. However the
> correct handling of EBUSY was missing and has been added as a return
> code indicating a successful transfer to the crypto engine.
> 
> Cc: [email protected] # 6.17+
> 
>> diff --git a/arch/s390/crypto/phmac_s390.c 
>> b/arch/s390/crypto/phmac_s390.c
>> index 03ca33ffe6ccf..30c8a59af1a4e 100644
>> --- a/arch/s390/crypto/phmac_s390.c
>> +++ b/arch/s390/crypto/phmac_s390.c
> 
> [ ... ]
> 
>> @@ -716,15 +726,17 @@ static int phmac_finup(struct ahash_request 
>> *req)
>>  		/* req->async_op has been set to either OP_FINUP or OP_FINAL */
>>  		atomic_inc(&tfm_ctx->via_engine_ctr);
>>  		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, 
>> req);
>> -		if (rc != -EINPROGRESS)
>> +		if (rc == -EINPROGRESS || rc == -EBUSY)
>> +			cleanup = false;
>> +		else
>>  			atomic_dec(&tfm_ctx->via_engine_ctr);
>>  	}
>> 
>> -	if (rc != -EINPROGRESS)
>> +	if (cleanup && hwh->walkbytes > 0)
>>  		hwh_advance(hwh, rc);
>> 
>>  out:
>> -	if (rc != -EINPROGRESS)
>> +	if (cleanup)
>>  		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));
>>  	pr_debug("rc=%d\n", rc);
>>  	return rc;
> 
> [Severity: High]
> This is a pre-existing issue, but while reviewing this file I noticed a
> potential module teardown order problem in s390_phmac_exit() in
> arch/s390/crypto/phmac_s390.c:
> 
>     if (phmac_crypto_engine) {
>         crypto_engine_stop(phmac_crypto_engine);
>         crypto_engine_exit(phmac_crypto_engine);
>     }
> 
>     for (i = ARRAY_SIZE(phmac_algs) - 1; i >= 0; i--) {
>         phmac = &phmac_algs[i];
>         if (phmac->registered)
>             crypto_engine_unregister_ahash(&phmac->alg);
>     }
> 
> Could this lead to kthread leaks and a use-after-free of the 
> crypto_engine?
> 
> If s390_phmac_init() fails to register a subsequent algorithm after
> successfully registering the first, it jumps to the error path and
> calls s390_phmac_exit().
> 
> Because the first algorithm is already registered and the module is in
> MODULE_STATE_COMING, a concurrent userspace program can successfully
> allocate it and continuously submit hash requests.
> 
> Since s390_phmac_exit() calls crypto_engine_stop() before unregistering
> the algorithms, the continuous stream of requests keeps the engine 
> busy.
> This can cause crypto_engine_stop() to time out and return -EBUSY,
> which leads to crypto_engine_exit() returning without destroying the
> kworker thread.
> 
> Finally, misc_deregister() destroys the device, freeing the 
> devm-managed
> crypto_engine struct, and the module is subsequently unloaded.
> 
> Would the still-running kworker thread then access freed memory and
> execute freed module code, potentially causing a kernel panic?

This will be addressed in another series. There is a new tracking 
bugzilla for this:
https://bugzilla.linux.ibm.com/show_bug.cgi?id=222134