RE: [EXT] [PATCH 1/2] crypto: caam - Fix wrong unmap operations

[Sahil Malhotra (OSS)]

Hi Changwei,

Honestly, I'd prefer an explicit request-type marker over inferring it from
key->d, but every cleaner option (e.g. adding FORM_PUBLIC=0 and switching on
priv_form everywhere) ripples across the driver and isn't worth it for a
targeted. So key->d check is ok from my side.
Please also add Fixes: bf53795025a2 ("crypto: caam - add crypto_engine support for RSA algorithms") in this commit.

Reviewed-by: Sahil Malhotra <sahil.malhotra@xxxxxxx>

Regards,
Sahil Malhotra


NXP Confidential
> -----Original Message-----
> From: Changwei Zou <changwei.zou@xxxxxxxxxxxxx>
> Sent: 21 August 2026 10:26
> To: Horia Geanta <horia.geanta@xxxxxxx>; Pankaj Gupta
> <pankaj.gupta@xxxxxxx>; gaurav.jain@xxxxxxx;
> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx
> Cc: linux-crypto@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> lukas@xxxxxxxxx; changwei.zou@xxxxxxxxxxxxx
> Subject: [EXT] [PATCH 1/2] crypto: caam - Fix wrong unmap operations
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Both akcipher_do_one_req() and akcipher_enqueue_req() call
> rsa_pub_unmap() regardless of the key type. priv_form only takes values
> FORM1/FORM2/FORM3 with no distinct public key enumerator.
>
> Use key->d to distinguish public from private key operations, then dispatch to
> the correct unmap function based on key->priv_form.
>
> caam_rsa_set_priv_key_form() implicitly relies on zero-initialization for
> priv_form. Set priv_form = FORM1 explicitly at the head of the function for
> clarity and robustness.
>
> Signed-off-by: Changwei Zou <changwei.zou@xxxxxxxxxxxxx>
> Assisted-by: OpenCode:claude-sonnet-4.6
> ---
>  drivers/crypto/caam/caampkc.c | 43 +++++++++++++++++++++++++--------
> --
>  1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> index cb001aa1de66..840271840cce 100644
> --- a/drivers/crypto/caam/caampkc.c
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -379,6 +379,7 @@ static int akcipher_do_one_req(struct crypto_engine
> *engine, void *areq)
>         struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
>         struct caam_rsa_req_ctx *req_ctx = akcipher_request_ctx(req);
>         struct caam_rsa_ctx *ctx = akcipher_tfm_ctx_dma(tfm);
> +       struct caam_rsa_key *key = &ctx->key;
>         struct device *jrdev = ctx->dev;
>         u32 *desc = req_ctx->edesc->hw_desc;
>         int ret;
> @@ -391,7 +392,21 @@ static int akcipher_do_one_req(struct crypto_engine
> *engine, void *areq)
>                 return ret;
>
>         if (ret != -EINPROGRESS) {
> -               rsa_pub_unmap(jrdev, req_ctx->edesc, req);
> +               if (key->d) {
> +                       switch (key->priv_form) {
> +                       case FORM1:
> +                               rsa_priv_f1_unmap(jrdev, req_ctx->edesc, req);
> +                               break;
> +                       case FORM2:
> +                               rsa_priv_f2_unmap(jrdev, req_ctx->edesc, req);
> +                               break;
> +                       case FORM3:
> +                               rsa_priv_f3_unmap(jrdev, req_ctx->edesc, req);
> +                               break;
> +                       }
> +               } else {
> +                       rsa_pub_unmap(jrdev, req_ctx->edesc, req);
> +               }
>                 rsa_io_unmap(jrdev, req_ctx->edesc, req);
>                 kfree(req_ctx->edesc);
>         } else {
> @@ -691,17 +706,19 @@ static int akcipher_enqueue_req(struct device
> *jrdev,
>                 ret = caam_jr_enqueue(jrdev, desc, cbk, req);
>
>         if ((ret != -EINPROGRESS) && (ret != -EBUSY)) {
> -               switch (key->priv_form) {
> -               case FORM1:
> -                       rsa_priv_f1_unmap(jrdev, edesc, req);
> -                       break;
> -               case FORM2:
> -                       rsa_priv_f2_unmap(jrdev, edesc, req);
> -                       break;
> -               case FORM3:
> -                       rsa_priv_f3_unmap(jrdev, edesc, req);
> -                       break;
> -               default:
> +               if (key->d) {
> +                       switch (key->priv_form) {
> +                       case FORM1:
> +                               rsa_priv_f1_unmap(jrdev, edesc, req);
> +                               break;
> +                       case FORM2:
> +                               rsa_priv_f2_unmap(jrdev, edesc, req);
> +                               break;
> +                       case FORM3:
> +                               rsa_priv_f3_unmap(jrdev, edesc, req);
> +                               break;
> +                       }
> +               } else {
>                         rsa_pub_unmap(jrdev, edesc, req);
>                 }
>                 rsa_io_unmap(jrdev, edesc, req); @@ -992,6 +1009,8 @@ static int
> caam_rsa_set_priv_key_form(struct caam_rsa_ctx *ctx,
>         size_t q_sz = raw_key->q_sz;
>         unsigned aligned_size;
>
> +       rsa_key->priv_form = FORM1;
> +
>         rsa_key->p = caam_read_raw_data(raw_key->p, &p_sz);
>         if (!rsa_key->p)
>                 return -ENOMEM;
> --
> 2.43.0
>