[PATCH] crypto: sa2ul - avoid rollback in sa_init_ctx_info()

[PATCH] crypto: sa2ul - avoid rollback in sa_init_ctx_info()

[Thorsten Blum]

Allocate the security context before reserving a security context ID to
avoid the rollback on allocation failure. Remove the now-unused local
error variable.

Signed-off-by: Thorsten Blum <blum@kernel.org>
---
 drivers/crypto/sa2ul.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index e026fc14a411..964ddadaa211 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c
@@ -748,7 +748,12 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,
 			    struct sa_crypto_data *data)
 {
 	unsigned long bn;
-	int err;
+
+	ctx->sc = dma_pool_alloc(data->sc_pool, GFP_KERNEL, &ctx->sc_phys);
+	if (!ctx->sc) {
+		dev_err(&data->pdev->dev, "Failed to allocate SC memory\n");
+		return -ENOMEM;
+	}
 
 	spin_lock(&data->scid_lock);
 	bn = find_first_zero_bit(data->ctx_bm, SA_MAX_NUM_CTX);
@@ -758,22 +763,7 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,
 
 	ctx->sc_id = (u16)(data->sc_id_start + bn);
 
-	ctx->sc = dma_pool_alloc(data->sc_pool, GFP_KERNEL, &ctx->sc_phys);
-	if (!ctx->sc) {
-		dev_err(&data->pdev->dev, "Failed to allocate SC memory\n");
-		err = -ENOMEM;
-		goto scid_rollback;
-	}
-
 	return 0;
-
-scid_rollback:
-	spin_lock(&data->scid_lock);
-	__clear_bit(bn, data->ctx_bm);
-	data->sc_id--;
-	spin_unlock(&data->scid_lock);
-
-	return err;
 }
 
 static void sa_cipher_cra_exit(struct crypto_skcipher *tfm)

Re: [PATCH] crypto: sa2ul - avoid rollback in sa_init_ctx_info()

[Chawdhry, Manorit]

Hi Thorsten,

On 11/4/2026 1:50 PM, Thorsten Blum wrote:
> Allocate the security context before reserving a security context ID to
> avoid the rollback on allocation failure. Remove the now-unused local
> error variable.
>
> Signed-off-by: Thorsten Blum <blum@kernel.org>
> ---
>   drivers/crypto/sa2ul.c | 22 ++++++----------------
>   1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
> index e026fc14a411..964ddadaa211 100644
> --- a/drivers/crypto/sa2ul.c
> +++ b/drivers/crypto/sa2ul.c
> @@ -748,7 +748,12 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,
>   			    struct sa_crypto_data *data)
>   {
>   	unsigned long bn;
> -	int err;
> +
> +	ctx->sc = dma_pool_alloc(data->sc_pool, GFP_KERNEL, &ctx->sc_phys);
> +	if (!ctx->sc) {
> +		dev_err(&data->pdev->dev, "Failed to allocate SC memory\n");
> +		return -ENOMEM;
> +	}

I have been working on a cleanup and the following patch [0] would 
actually defeat
the purpose of the patch you've posted, with the addition of the check, 
we'd have to
rollback this dma alloc'ed pool instead which just reverses the order of 
rollback and
nothing else.

Are you okay if we drop this patch ( given am not seeing any criticality 
) or do you see
it still helping after the context of the patch I shared?

Regards,
Manorit

>   
>   	spin_lock(&data->scid_lock);
>   	bn = find_first_zero_bit(data->ctx_bm, SA_MAX_NUM_CTX);
> @@ -758,22 +763,7 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,
>   
>   	ctx->sc_id = (u16)(data->sc_id_start + bn);
>   
> -	ctx->sc = dma_pool_alloc(data->sc_pool, GFP_KERNEL, &ctx->sc_phys);
> -	if (!ctx->sc) {
> -		dev_err(&data->pdev->dev, "Failed to allocate SC memory\n");
> -		err = -ENOMEM;
> -		goto scid_rollback;
> -	}
> -
>   	return 0;
> -
> -scid_rollback:
> -	spin_lock(&data->scid_lock);
> -	__clear_bit(bn, data->ctx_bm);
> -	data->sc_id--;
> -	spin_unlock(&data->scid_lock);
> -
> -	return err;
>   }
>   
>   static void sa_cipher_cra_exit(struct crypto_skcipher *tfm)

[0]: 
https://lore.kernel.org/linux-crypto/20261104-b4-upstream-sa2ul-cleanup-v2-21-15ab90b3f9c2@ti.com/

Re: [PATCH] crypto: sa2ul - avoid rollback in sa_init_ctx_info()

[Thorsten Blum]

Hi Manorit,

On Wed, Nov 04, 2026 at 01:06:15PM +0000, Chawdhry, Manorit wrote:
> Hi Thorsten,
> 
> On 11/4/2026 1:50 PM, Thorsten Blum wrote:
> > Allocate the security context before reserving a security context ID to
> > avoid the rollback on allocation failure. Remove the now-unused local
> > error variable.
> >
> > Signed-off-by: Thorsten Blum <blum@kernel.org>
> > ---
> >   drivers/crypto/sa2ul.c | 22 ++++++----------------
> >   1 file changed, 6 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
> > index e026fc14a411..964ddadaa211 100644
> > --- a/drivers/crypto/sa2ul.c
> > +++ b/drivers/crypto/sa2ul.c
> > @@ -748,7 +748,12 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,
> >   			    struct sa_crypto_data *data)
> >   {
> >   	unsigned long bn;
> > -	int err;
> > +
> > +	ctx->sc = dma_pool_alloc(data->sc_pool, GFP_KERNEL, &ctx->sc_phys);
> > +	if (!ctx->sc) {
> > +		dev_err(&data->pdev->dev, "Failed to allocate SC memory\n");
> > +		return -ENOMEM;
> > +	}
> 
> I have been working on a cleanup and the following patch [0] would 
> actually defeat
> the purpose of the patch you've posted, with the addition of the check, 
> we'd have to
> rollback this dma alloc'ed pool instead which just reverses the order of 
> rollback and
> nothing else.
> 
> Are you okay if we drop this patch ( given am not seeing any criticality 
> ) or do you see
> it still helping after the context of the patch I shared?

You would need to call dma_pool_free() with my patch, but it's probably
better to not allocate any memory at all. So yes, feel free to drop it.

Thanks,
Thorsten