On Sat, 2019-07-27 at 11:03 +0000, Bernard Metzler wrote:
> -----"Dan Carpenter" <dan.carpenter@xxxxxxxxxx> wrote: -----
>
> > To: bmt@xxxxxxxxxxxxxx
> > From: "Dan Carpenter" <dan.carpenter@xxxxxxxxxx>
> > Date: 07/26/2019 10:11AM
> > Cc: linux-rdma@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
