[patch] IB/ipath: add a range check before bumping the stats

[patch] IB/ipath: add a range check before bumping the stats

[Dan Carpenter]

My static checker complains that this could corrupt memory.  It
seems valid to add a range check here.

Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
I can't test this.

diff --git a/drivers/infiniband/hw/ipath/ipath_verbs.c b/drivers/infiniband/hw/ipath/ipath_verbs.c
index 439c35d..f15dd70 100644
--- a/drivers/infiniband/hw/ipath/ipath_verbs.c
+++ b/drivers/infiniband/hw/ipath/ipath_verbs.c
@@ -621,6 +621,10 @@ void ipath_ib_rcv(struct ipath_ibdev *dev, void *rhdr, void *data,
 	}
 
 	opcode = be32_to_cpu(ohdr->bth[0]) >> 24;
+	if (opcode >= ARRAY_SIZE(dev->opstats)) {
+		dev->rcv_errors++;
+		goto bail;
+	}
 	dev->opstats[opcode].n_bytes += tlen;
 	dev->opstats[opcode].n_packets++;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [patch] IB/ipath: add a range check before bumping the stats

[Marciniszyn, Mike]

Oddly, I don't see an issue with qib, which masks the opcode.

Would:
opcode = (be32_to_cpu(ohdr->bth[0]) >> 24) & 0x7f;

also correct the issue without adding the if test.   At this point the hardware has already validated the opcode, so there is no value in the fail handling.

What args are you using with smatch?

Mike

> My static checker complains that this could corrupt memory.  It seems valid to
> add a range check here.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] IB/ipath: add a range check before bumping the stats

[Dan Carpenter]

On Fri, Mar 15, 2013 at 01:43:39PM +0000, Marciniszyn, Mike wrote:
> Oddly, I don't see an issue with qib, which masks the opcode.
> 
> Would:
> opcode = (be32_to_cpu(ohdr->bth[0]) >> 24) & 0x7f;
> 
> also correct the issue without adding the if test.   At this point
> the hardware has already validated the opcode, so there is no
> value in the fail handling.
> 
> What args are you using with smatch?
> 

Gar sorry, for the noise.  You can just drop this then.

This is some custom stuff that probably has too many false positives
to push.

Doing an "& 0x7f" would silence the warning but if this is validated
by the hardware then we don't need it.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html