Received-SPF: pass (google.com: domain of linux-crypto+bounces-460-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20231201124929.12448-1-n.zhandarovich@fintech.ru>
References: <20231201124929.12448-1-n.zhandarovich@fintech.ru>
Subject: Re: [PATCH v2] crypto: safexcel - Add error handling for dma_map_sg() calls
From: Antoine Tenart <atenart@kernel.org>
Cc: Nikita Zhandarovich <n.zhandarovich@fintech.ru>, David S. Miller <davem@davemloft.net>, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
To: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Date: Fri, 01 Dec 2023 14:39:47 +0100
Message-ID: <170143798791.35162.11594364771392098948@kwain>

Quoting Nikita Zhandarovich (2023-12-01 13:49:29)
> Macro dma_map_sg() may return 0 on error. This patch enables
> checks in case of the macro failure and ensures unmapping of
> previously mapped buffers with dma_unmap_sg().
>=20
> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>=20
> Fixes: 49186a7d9e46 ("crypto: inside_secure - Avoid dma map if size is ze=
ro")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>

Reviewed-by: Antoine Tenart <atenart@kernel.org>

Thanks!
Antoine

> ---
> v2: remove extra level of parentheses and
> change return error code from -ENOMEM to EIO
> per Antoine Tenart's <atenart@kernel.org> suggestion
>=20
>  drivers/crypto/inside-secure/safexcel_cipher.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>=20
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/cry=
pto/inside-secure/safexcel_cipher.c
> index 272c28b5a088..b83818634ae4 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -742,9 +742,9 @@ static int safexcel_send_req(struct crypto_async_requ=
est *base, int ring,
>                                 max(totlen_src, totlen_dst));
>                         return -EINVAL;
>                 }
> -               if (sreq->nr_src > 0)
> -                       dma_map_sg(priv->dev, src, sreq->nr_src,
> -                                  DMA_BIDIRECTIONAL);
> +               if (sreq->nr_src > 0 &&
> +                   !dma_map_sg(priv->dev, src, sreq->nr_src, DMA_BIDIREC=
TIONAL))
> +                       return -EIO;
>         } else {
>                 if (unlikely(totlen_src && (sreq->nr_src <=3D 0))) {
>                         dev_err(priv->dev, "Source buffer not large enoug=
h (need %d bytes)!",
> @@ -752,8 +752,9 @@ static int safexcel_send_req(struct crypto_async_requ=
est *base, int ring,
>                         return -EINVAL;
>                 }
> =20
> -               if (sreq->nr_src > 0)
> -                       dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_D=
EVICE);
> +               if (sreq->nr_src > 0 &&
> +                   !dma_map_sg(priv->dev, src, sreq->nr_src, DMA_TO_DEVI=
CE))
> +                       return -EIO;
> =20
>                 if (unlikely(totlen_dst && (sreq->nr_dst <=3D 0))) {
>                         dev_err(priv->dev, "Dest buffer not large enough =
(need %d bytes)!",
> @@ -762,9 +763,11 @@ static int safexcel_send_req(struct crypto_async_req=
uest *base, int ring,
>                         goto unmap;
>                 }
> =20
> -               if (sreq->nr_dst > 0)
> -                       dma_map_sg(priv->dev, dst, sreq->nr_dst,
> -                                  DMA_FROM_DEVICE);
> +               if (sreq->nr_dst > 0 &&
> +                   !dma_map_sg(priv->dev, dst, sreq->nr_dst, DMA_FROM_DE=
VICE)) {
> +                       ret =3D -EIO;
> +                       goto unmap;
> +               }
>         }
> =20
>         memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);
>