All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] rdma/siw: queue pair methods
@ 2019-07-26  8:10 Dan Carpenter
  2019-07-27 11:03 ` Bernard Metzler
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-07-26  8:10 UTC (permalink / raw)
  To: bmt; +Cc: linux-rdma

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] rdma/siw: queue pair methods
  2019-07-26  8:10 [bug report] rdma/siw: queue pair methods Dan Carpenter
@ 2019-07-27 11:03 ` Bernard Metzler
  2019-07-29 17:36   ` Doug Ledford
  0 siblings, 1 reply; 3+ messages in thread
From: Bernard Metzler @ 2019-07-27 11:03 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-rdma

-----"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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [bug report] rdma/siw: queue pair methods
  2019-07-27 11:03 ` Bernard Metzler
@ 2019-07-29 17:36   ` Doug Ledford
  0 siblings, 0 replies; 3+ messages in thread
From: Doug Ledford @ 2019-07-29 17:36 UTC (permalink / raw)
  To: Bernard Metzler, Dan Carpenter; +Cc: linux-rdma

[-- Attachment #1: Type: text/plain, Size: 4172 bytes --]

On Sat, 2019-07-27 at 11:03 +0000, Bernard Metzler wrote:
> -----"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:
> 

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).

> 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) {

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-07-29 17:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-26  8:10 [bug report] rdma/siw: queue pair methods Dan Carpenter
2019-07-27 11:03 ` Bernard Metzler
2019-07-29 17:36   ` Doug Ledford

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.