Re: [bug report] rdma/siw: queue pair methods

From: "Bernard Metzler" <BMT@zurich.ibm.com>
To: "Dan Carpenter" <dan.carpenter@oracle.com>
Cc: linux-rdma@vger.kernel.org
Subject: Re: [bug report] rdma/siw: queue pair methods
Date: Sat, 27 Jul 2019 11:03:55 +0000	[thread overview]
Message-ID: <OF61E386ED.49A73798-ON00258444.003BD6A6-00258444.003CC8D9@notes.na.collabserv.com> (raw)
In-Reply-To: <20190726081056.GA27059@mwanda>

-----"Dan Carpenter" <dan.carpenter@oracle.com> wrote: -----

>To: bmt@zurich.ibm.com
>From: "Dan Carpenter" <dan.carpenter@oracle.com>
>Date: 07/26/2019 10:11AM
>Cc: linux-rdma@vger.kernel.org
>Subject: [EXTERNAL] [bug report] rdma/siw: queue pair methods
>
>Hello Bernard Metzler,
>
>The patch f29dd55b0236: "rdma/siw: queue pair methods" from Jun 20,
>2019, leads to the following static checker warning:
>
>	drivers/infiniband/sw/siw/siw_qp.c:226 siw_qp_enable_crc()
>	warn: variable dereferenced before check 'siw_crypto_shash' (see
>line 223)
>
>drivers/infiniband/sw/siw/siw_qp.c
>   219  static int siw_qp_enable_crc(struct siw_qp *qp)
>   220  {
>   221          struct siw_rx_stream *c_rx = &qp->rx_stream;
>   222          struct siw_iwarp_tx *c_tx = &qp->tx_ctx;
>   223          int size = crypto_shash_descsize(siw_crypto_shash) +
>                                                 ^^^^^^^^^^^^^^^^
>Dereferenced inside function.
>
>   224                          sizeof(struct shash_desc);
>   225  
>   226          if (siw_crypto_shash == NULL)
>                    ^^^^^^^^^^^^^^^^^^^^^^^^
>Checked too late.
>
>   227                  return -ENOENT;
>   228  
>   229          c_tx->mpa_crc_hd = kzalloc(size, GFP_KERNEL);
>   230          c_rx->mpa_crc_hd = kzalloc(size, GFP_KERNEL);
>   231          if (!c_tx->mpa_crc_hd || !c_rx->mpa_crc_hd) {
>   232                  kfree(c_tx->mpa_crc_hd);
>   233                  kfree(c_rx->mpa_crc_hd);
>   234                  c_tx->mpa_crc_hd = NULL;
>   235                  c_rx->mpa_crc_hd = NULL;
>   236                  return -ENOMEM;
>   237          }
>   238          c_tx->mpa_crc_hd->tfm = siw_crypto_shash;
>   239          c_rx->mpa_crc_hd->tfm = siw_crypto_shash;
>   240  
>   241          return 0;
>   242  }
>
>regards,
>dan carpenter
>
>

Hi Dan,
many thanks for catching this one! The fix of course is simple:


From c13b5da99aea7766a61aabe33e9943618f4505cf Mon Sep 17 00:00:00 2001
From: Bernard Metzler <bmt@zurich.ibm.com>
Date: Sat, 27 Jul 2019 12:38:32 +0200
Subject: [PATCH] Do not dereference 'siw_crypto_shash' before checking

Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_qp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_qp.c b/drivers/infiniband/sw/siw/siw_qp.c
index 11383d9f95ef..e27bd5b35b96 100644
--- a/drivers/infiniband/sw/siw/siw_qp.c
+++ b/drivers/infiniband/sw/siw/siw_qp.c
@@ -220,12 +220,14 @@ static int siw_qp_enable_crc(struct siw_qp *qp)
 {
 	struct siw_rx_stream *c_rx = &qp->rx_stream;
 	struct siw_iwarp_tx *c_tx = &qp->tx_ctx;
-	int size = crypto_shash_descsize(siw_crypto_shash) +
-			sizeof(struct shash_desc);
+	int size;
 
 	if (siw_crypto_shash == NULL)
 	return -ENOENT;
 
+	size = crypto_shash_descsize(siw_crypto_shash) +
+		sizeof(struct shash_desc);
+
 	c_tx->mpa_crc_hd = kzalloc(size, GFP_KERNEL);
 	c_rx->mpa_crc_hd = kzalloc(size, GFP_KERNEL);
 	if (!c_tx->mpa_crc_hd || !c_rx->mpa_crc_hd) {
-- 
2.17.2

