Re: [PATCH v2] crypto: ccp - Fix use-after-free in backlog cmd advancement

[Herbert Xu]

On Tue, Sep 29, 2026 at 02:38:25PM +0000, Fan Wu wrote:
>
> @@ -391,26 +372,61 @@ static struct ccp_cmd *ccp_dequeue_cmd(struct ccp_cmd_queue *cmd_q)
>  		return NULL;
>  	}
>  
> -	if (ccp->cmd_count) {
> +	if (!list_empty(&ccp->cmd)) {
>  		cmd_q->active = 1;
>  
>  		cmd = list_first_entry(&ccp->cmd, struct ccp_cmd, entry);
>  		list_del(&cmd->entry);
>  
> -		ccp->cmd_count--;
> -	}
> -
> -	if (!list_empty(&ccp->backlog)) {
> +		if (!list_empty(&ccp->backlog)) {
> +			/* Transfer the freed slot to the backlogged
> +			 * command, so that concurrent submissions cannot
> +			 * claim the space meant for it.
> +			 */
> +			backlog = list_first_entry(&ccp->backlog,
> +						   struct ccp_cmd, entry);
> +			list_del(&backlog->entry);
> +		} else {
> +			ccp->cmd_count--;
> +		}
> +	} else if (!list_empty(&ccp->backlog)) {
> +		/* No command is available for execution, but a backlogged
> +		 * command is stranded: reserve a slot for it.
> +		 */
>  		backlog = list_first_entry(&ccp->backlog, struct ccp_cmd,
>  					   entry);
>  		list_del(&backlog->entry);
> +
> +		ccp->cmd_count++;

This seems dangerous.  Since you're keeping the cmd_count without
actually moving the backlogged entry over to the active list.
Wouldn't this increment potentially exceed the limit on the number
of active entries?

I think rather than spending time on fixing this mess, we should
just convert it over to crypto_engine and be done with it.  The
crypto_engine system is known to work without these subtle race
conditions and all drivers should use it.

Thanks,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt