2021-01-04 17:23:12

by Cabiddu, Giovanni

[permalink] [raw]
Subject: [PATCH 0/3] crypto: qat - fix issues reported by smatch

Fix warnings and errors reported by the static analysis tool smatch in
the QAT driver.

Adam Guerin (3):
crypto: qat - fix potential spectre issue
crypto: qat - change format string and cast ring size
crypto: qat - reduce size of mapped region

drivers/crypto/qat/qat_common/adf_transport.c | 2 ++
drivers/crypto/qat/qat_common/adf_transport_debug.c | 4 ++--
drivers/crypto/qat/qat_common/qat_asym_algs.c | 12 ++++++------
3 files changed, 10 insertions(+), 8 deletions(-)

--
2.29.2


2021-01-04 17:23:34

by Cabiddu, Giovanni

[permalink] [raw]
Subject: [PATCH 1/3] crypto: qat - fix potential spectre issue

From: Adam Guerin <[email protected]>

Sanitize ring_num value coming from configuration (and potentially
from user space) before it is used as index in the banks array.

This issue was detected by smatch:

drivers/crypto/qat/qat_common/adf_transport.c:233 adf_create_ring() warn: potential spectre issue 'bank->rings' [r] (local cap)

Signed-off-by: Adam Guerin <[email protected]>
Reviewed-by: Giovanni Cabiddu <[email protected]>
Signed-off-by: Giovanni Cabiddu <[email protected]>
---
drivers/crypto/qat/qat_common/adf_transport.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/qat/qat_common/adf_transport.c b/drivers/crypto/qat/qat_common/adf_transport.c
index 5a7030acdc33..888c1e047295 100644
--- a/drivers/crypto/qat/qat_common/adf_transport.c
+++ b/drivers/crypto/qat/qat_common/adf_transport.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0-only)
/* Copyright(c) 2014 - 2020 Intel Corporation */
#include <linux/delay.h>
+#include <linux/nospec.h>
#include "adf_accel_devices.h"
#include "adf_transport_internal.h"
#include "adf_transport_access_macros.h"
@@ -246,6 +247,7 @@ int adf_create_ring(struct adf_accel_dev *accel_dev, const char *section,
return -EFAULT;
}

+ ring_num = array_index_nospec(ring_num, num_rings_per_bank);
bank = &transport_data->banks[bank_num];
if (adf_reserve_ring(bank, ring_num)) {
dev_err(&GET_DEV(accel_dev), "Ring %d, %s already exists.\n",
--
2.29.2

2021-01-04 17:23:35

by Cabiddu, Giovanni

[permalink] [raw]
Subject: [PATCH 2/3] crypto: qat - change format string and cast ring size

From: Adam Guerin <[email protected]>

Cast ADF_SIZE_TO_RING_SIZE_IN_BYTES() so it can return a 64 bit value.

This issue was detected by smatch:

drivers/crypto/qat/qat_common/adf_transport_debug.c:65 adf_ring_show() warn: should '(1 << (ring->ring_size - 1)) << 7' be a 64 bit type?

Signed-off-by: Adam Guerin <[email protected]>
Reviewed-by: Giovanni Cabiddu <[email protected]>
Signed-off-by: Giovanni Cabiddu <[email protected]>
---
drivers/crypto/qat/qat_common/adf_transport_debug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_transport_debug.c b/drivers/crypto/qat/qat_common/adf_transport_debug.c
index 1205186ad51e..e69e5907f595 100644
--- a/drivers/crypto/qat/qat_common/adf_transport_debug.c
+++ b/drivers/crypto/qat/qat_common/adf_transport_debug.c
@@ -62,8 +62,8 @@ static int adf_ring_show(struct seq_file *sfile, void *v)
seq_printf(sfile, "head %x, tail %x, empty: %d\n",
head, tail, (empty & 1 << ring->ring_number)
>> ring->ring_number);
- seq_printf(sfile, "ring size %d, msg size %d\n",
- ADF_SIZE_TO_RING_SIZE_IN_BYTES(ring->ring_size),
+ seq_printf(sfile, "ring size %lld, msg size %d\n",
+ (long long)ADF_SIZE_TO_RING_SIZE_IN_BYTES(ring->ring_size),
ADF_MSG_SIZE_TO_BYTES(ring->msg_size));
seq_puts(sfile, "----------- Ring data ------------\n");
return 0;
--
2.29.2

2021-01-04 17:23:36

by Cabiddu, Giovanni

[permalink] [raw]
Subject: [PATCH 3/3] crypto: qat - reduce size of mapped region

From: Adam Guerin <[email protected]>

Restrict size of field to what is required by the operation.

This issue was detected by smatch:

drivers/crypto/qat/qat_common/qat_asym_algs.c:328 qat_dh_compute_value() error: dma_map_single_attrs() '&qat_req->in.dh.in.b' too small (8 vs 64)

Signed-off-by: Adam Guerin <[email protected]>
Reviewed-by: Giovanni Cabiddu <[email protected]>
Signed-off-by: Giovanni Cabiddu <[email protected]>
---
drivers/crypto/qat/qat_common/qat_asym_algs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index 2c863d25327a..b0b78445418b 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -321,13 +321,13 @@ static int qat_dh_compute_value(struct kpp_request *req)
qat_req->out.dh.out_tab[1] = 0;
/* Mapping in.in.b or in.in_g2.xa is the same */
qat_req->phy_in = dma_map_single(dev, &qat_req->in.dh.in.b,
- sizeof(struct qat_dh_input_params),
+ sizeof(qat_req->in.dh.in.b),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_in)))
goto unmap_dst;

qat_req->phy_out = dma_map_single(dev, &qat_req->out.dh.r,
- sizeof(struct qat_dh_output_params),
+ sizeof(qat_req->out.dh.r),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_out)))
goto unmap_in_params;
@@ -716,13 +716,13 @@ static int qat_rsa_enc(struct akcipher_request *req)
qat_req->in.rsa.in_tab[3] = 0;
qat_req->out.rsa.out_tab[1] = 0;
qat_req->phy_in = dma_map_single(dev, &qat_req->in.rsa.enc.m,
- sizeof(struct qat_rsa_input_params),
+ sizeof(qat_req->in.rsa.enc.m),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_in)))
goto unmap_dst;

qat_req->phy_out = dma_map_single(dev, &qat_req->out.rsa.enc.c,
- sizeof(struct qat_rsa_output_params),
+ sizeof(qat_req->out.rsa.enc.c),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_out)))
goto unmap_in_params;
@@ -864,13 +864,13 @@ static int qat_rsa_dec(struct akcipher_request *req)
qat_req->in.rsa.in_tab[3] = 0;
qat_req->out.rsa.out_tab[1] = 0;
qat_req->phy_in = dma_map_single(dev, &qat_req->in.rsa.dec.c,
- sizeof(struct qat_rsa_input_params),
+ sizeof(qat_req->in.rsa.dec.c),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_in)))
goto unmap_dst;

qat_req->phy_out = dma_map_single(dev, &qat_req->out.rsa.dec.m,
- sizeof(struct qat_rsa_output_params),
+ sizeof(qat_req->out.rsa.dec.m),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, qat_req->phy_out)))
goto unmap_in_params;
--
2.29.2

2021-01-14 06:47:56

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] crypto: qat - fix issues reported by smatch

On Mon, Jan 04, 2021 at 05:21:56PM +0000, Giovanni Cabiddu wrote:
> Fix warnings and errors reported by the static analysis tool smatch in
> the QAT driver.
>
> Adam Guerin (3):
> crypto: qat - fix potential spectre issue
> crypto: qat - change format string and cast ring size
> crypto: qat - reduce size of mapped region
>
> drivers/crypto/qat/qat_common/adf_transport.c | 2 ++
> drivers/crypto/qat/qat_common/adf_transport_debug.c | 4 ++--
> drivers/crypto/qat/qat_common/qat_asym_algs.c | 12 ++++++------
> 3 files changed, 10 insertions(+), 8 deletions(-)

All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt