rdma/siw: queue pair methods

bug report Dan Carpenter 3 participants 3 messages Jul 26, 2019 – Jul 29, 2019

Subsystems (2)

InfiniBand Soft-iWARP driver (siw)

lore

Share & tools

lore

Bernard Metzler

2019-07-27 11:04 UTC #1 lore raw

-----"Dan Carpenter" <[email protected]> wrote: -----

Quoted 47 lines

To: [email protected]
From: "Dan Carpenter" <[email protected]>
Date: 07/26/2019 10:11AM
Cc: [email protected]
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  }

Quoted 4 lines

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 <[email protected]>
Date: Sat, 27 Jul 2019 12:38:32 +0200
Subject: [PATCH] Do not dereference 'siw_crypto_shash' before checking

Signed-off-by Bernard Metzler <[email protected]>

---
 drivers/infiniband/sw/siw/siw_qp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/infiniband/sw/siw/siw_qp.c +4 -2

index 11383d9f95ef..e27bd5b35b96 100644
@@ -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) {

Signature · 1 line

2.17.2

Doug Ledford

2019-07-29 17:36 UTC #2 lore raw

Quoted 49 lines · Bernard Metzler

-----"Dan Carpenter" <[email protected]> wrote: -----
Quoted 47 lines
To: [email protected]
From: "Dan Carpenter" <[email protected]>
Date: 07/26/2019 10:11AM
Cc: [email protected]
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  }

Quoted 8 lines

Quoted 4 lines
regards,
dan carpenter
Hi Dan,
many thanks for catching this one! The fix of course is simple:

Hi Bernard,

This patch was ignored by patchworks for some reason. If I hadn't noticed that it was here, but not in patchworks and also not applied previously by Jason, it would have been missed entirely. I suspect it's because the patch was embedded in a reply, but I'm not sure as that normally seems to work. In any case, I might suggest next time you reply to the bug report that you have a fix, and then use git send-email to send the patch, just to be on the safe side in terms of things getting lost.

With all that said, applied to for-rc along with some fix ups to the log message (added Reported-by: and Fixes: tags).

Quoted 32 lines

Signed-off-by Bernard Metzler <[email protected]>

---
 drivers/infiniband/sw/siw/siw_qp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/infiniband/sw/siw/siw_qp.c +4 -2

index 11383d9f95ef..e27bd5b35b96 100644
@@ -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) {

Signature · 3 lines

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD