[PATCH] RDMA/cxgb4: Add a sanity check in process_work()

[PATCH] RDMA/cxgb4: Add a sanity check in process_work()

[Dan Carpenter]

The story is that Smatch marks skb->data as untrusted so it generates
a warning message here:

    drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
    error: buffer overflow 'work_handlers' 241 <= 255

In other places which handle this such as t4_uld_rx_handler() there is
some checking to make sure that the function pointer is not NULL.  I
have added bounds checking and a check for NULL here as well.

Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 21db3b48a617..844c9e78df8b 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -4097,9 +4097,15 @@ static void process_work(struct work_struct *work)
 		dev = *((struct c4iw_dev **) (skb->cb + sizeof(void *)));
 		opcode = rpl->ot.opcode;
 
-		ret = work_handlers[opcode](dev, skb);
-		if (!ret)
+		if (opcode >= ARRAY_SIZE(work_handlers) ||
+		    !work_handlers[opcode]) {
+			pr_err("No handler for opcode 0x%x.\n", opcode);
 			kfree_skb(skb);
+		} else {
+			ret = work_handlers[opcode](dev, skb);
+			if (!ret)
+				kfree_skb(skb);
+		}
 		process_timedout_eps();
 	}
 }
--
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

[PATCH] RDMA/cxgb4: Add a sanity check in process_work()

[Dan Carpenter]

The story is that Smatch marks skb->data as untrusted so it generates
a warning message here:

    drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
    error: buffer overflow 'work_handlers' 241 <= 255

In other places which handle this such as t4_uld_rx_handler() there is
some checking to make sure that the function pointer is not NULL.  I
have added bounds checking and a check for NULL here as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 21db3b48a617..844c9e78df8b 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -4097,9 +4097,15 @@ static void process_work(struct work_struct *work)
 		dev = *((struct c4iw_dev **) (skb->cb + sizeof(void *)));
 		opcode = rpl->ot.opcode;
 
-		ret = work_handlers[opcode](dev, skb);
-		if (!ret)
+		if (opcode >= ARRAY_SIZE(work_handlers) ||
+		    !work_handlers[opcode]) {
+			pr_err("No handler for opcode 0x%x.\n", opcode);
 			kfree_skb(skb);
+		} else {
+			ret = work_handlers[opcode](dev, skb);
+			if (!ret)
+				kfree_skb(skb);
+		}
 		process_timedout_eps();
 	}
 }

RE: [PATCH] RDMA/cxgb4: Add a sanity check in process_work()

[Steve Wise]

> 
> The story is that Smatch marks skb->data as untrusted so it generates
> a warning message here:
> 
>     drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
>     error: buffer overflow 'work_handlers' 241 <= 255
> 
> In other places which handle this such as t4_uld_rx_handler() there is
> some checking to make sure that the function pointer is not NULL.  I
> have added bounds checking and a check for NULL here as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Acked-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>


--
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] RDMA/cxgb4: Add a sanity check in process_work()

[Steve Wise]

> 
> The story is that Smatch marks skb->data as untrusted so it generates
> a warning message here:
> 
>     drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
>     error: buffer overflow 'work_handlers' 241 <= 255
> 
> In other places which handle this such as t4_uld_rx_handler() there is
> some checking to make sure that the function pointer is not NULL.  I
> have added bounds checking and a check for NULL here as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Steve Wise <swise@opengridcomputing.com>

Re: RDMA/cxgb4: Add a sanity check in process_work()

[Jason Gunthorpe]

On Tue, Dec 05, 2017 at 05:36:54PM +0300, Dan Carpenter wrote:
> The story is that Smatch marks skb->data as untrusted so it generates
> a warning message here:
> 
>     drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
>     error: buffer overflow 'work_handlers' 241 <= 255
> 
> In other places which handle this such as t4_uld_rx_handler() there is
> some checking to make sure that the function pointer is not NULL.  I
> have added bounds checking and a check for NULL here as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Acked-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>

Thanks, applied to -next

Jason
--
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: RDMA/cxgb4: Add a sanity check in process_work()

[Jason Gunthorpe]

On Tue, Dec 05, 2017 at 05:36:54PM +0300, Dan Carpenter wrote:
> The story is that Smatch marks skb->data as untrusted so it generates
> a warning message here:
> 
>     drivers/infiniband/hw/cxgb4/cm.c:4100 process_work()
>     error: buffer overflow 'work_handlers' 241 <= 255
> 
> In other places which handle this such as t4_uld_rx_handler() there is
> some checking to make sure that the function pointer is not NULL.  I
> have added bounds checking and a check for NULL here as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Acked-by: Steve Wise <swise@opengridcomputing.com>

Thanks, applied to -next

Jason