Skip to contentSkip to conversation

RDMA/iw_cxgb4: Handle return value of c4iw_ofld_send() in abort_arp_failure()

bug report Dan Carpenter 2 participants 3 messages Feb 23, 2017 – Apr 13, 2017 InfiniBand

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Hariprasad S,

The patch 761e19a504af: "RDMA/iw_cxgb4: Handle return value of c4iw_ofld_send() in abort_arp_failure()" from May 6, 2016, leads to the following static checker warning:

drivers/infiniband/hw/cxgb4/cm.c:575 abort_arp_failure()
warn: passing freed memory 'skb'

drivers/infiniband/hw/cxgb4/cm.c
559 /*

   560   * Handle an ARP failure for a CPL_ABORT_REQ.  Change it into a no RST variant
   561   * and send it along.
   562   */
   563  static void abort_arp_failure(void *handle, struct sk_buff *skb)
   564  {
   565          int ret;
   566          struct c4iw_ep *ep = handle;
   567          struct c4iw_rdev *rdev = &ep->com.dev->rdev;
   568          struct cpl_abort_req *req = cplhdr(skb);
   569  
   570          PDBG("%s rdev %p\n", __func__, rdev);
   571          req->cmd = CPL_ABORT_NO_RST;
   572          ret = c4iw_ofld_send(rdev, skb);
   573          if (ret) {

If c4iw_ofld_send() fails then it frees skb.

574 __state_set(&ep->com, DEAD);

   575                  queue_arp_failure_cpl(ep, skb, FAKE_CPL_PUT_EP_SAFE);
                                                  ^^^

Used after free here. These are ref counted data so it might actually be that I have misread it and miscounted the references so perhaps it's OK, but it looks might suspect at a glance.

576 }
577 }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#1 lore raw

Any update on this? It really looks buggy to me.

regards,
dan carpenter

Quoted 35 lines · Dan Carpenter

Hello Hariprasad S,

The patch 761e19a504af: "RDMA/iw_cxgb4: Handle return value of c4iw_ofld_send() in abort_arp_failure()" from May 6, 2016, leads to the following static checker warning:

drivers/infiniband/hw/cxgb4/cm.c:575 abort_arp_failure()
warn: passing freed memory 'skb'

drivers/infiniband/hw/cxgb4/cm.c
559 /*

   560   * Handle an ARP failure for a CPL_ABORT_REQ.  Change it into a no RST variant
   561   * and send it along.
   562   */
   563  static void abort_arp_failure(void *handle, struct sk_buff *skb)
   564  {
   565          int ret;
   566          struct c4iw_ep *ep = handle;
   567          struct c4iw_rdev *rdev = &ep->com.dev->rdev;
   568          struct cpl_abort_req *req = cplhdr(skb);
   569  
   570          PDBG("%s rdev %p\n", __func__, rdev);
   571          req->cmd = CPL_ABORT_NO_RST;
   572          ret = c4iw_ofld_send(rdev, skb);
   573          if (ret) {

If c4iw_ofld_send() fails then it frees skb.

   574                  __state_set(&ep->com, DEAD);
   575                  queue_arp_failure_cpl(ep, skb, FAKE_CPL_PUT_EP_SAFE);
                                                  ^^^

Used after free here. These are ref counted data so it might actually be that I have misread it and miscounted the references so perhaps it's OK, but it looks might suspect at a glance.

Quoted 5 lines
   576          }
   577  }

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 2 lines

Any update on this? It really looks buggy to me.

Hey Dan, I think you're correct. Seems like abort_arp_failure() needs to take a ref before calling cxgb4_ofld_send() because cxgb4_ofld_send() will free the skb if there is a fatal error. So abort_arp_failure() will deref the skb if cxgb4_ofld_send() suceeds. Otherwise, the deref will happen in _put_ep_safe(). The same issue exists with FAKE_CPL_PASS_PUT_EP_SAFE too, and maybe all the arp handlers.

Steve.

Quoted 16 lines

regards,
dan carpenter

Quoted 12 lines · Dan Carpenter

Hello Hariprasad S,

The patch 761e19a504af: "RDMA/iw_cxgb4: Handle return value of c4iw_ofld_send() in abort_arp_failure()" from May 6, 2016, leads to the following static checker warning:

drivers/infiniband/hw/cxgb4/cm.c:575 abort_arp_failure()
warn: passing freed memory 'skb'

drivers/infiniband/hw/cxgb4/cm.c
559 /*
560 * Handle an ARP failure for a CPL_ABORT_REQ. Change it into a no

RST

Quoted 19 lines

variant

Quoted 18 lines
   561   * and send it along.
   562   */
   563  static void abort_arp_failure(void *handle, struct sk_buff *skb)
   564  {
   565          int ret;
   566          struct c4iw_ep *ep = handle;
   567          struct c4iw_rdev *rdev = &ep->com.dev->rdev;
   568          struct cpl_abort_req *req = cplhdr(skb);
   569
   570          PDBG("%s rdev %p\n", __func__, rdev);
   571          req->cmd = CPL_ABORT_NO_RST;
   572          ret = c4iw_ofld_send(rdev, skb);
   573          if (ret) {

If c4iw_ofld_send() fails then it frees skb.

   574                  __state_set(&ep->com, DEAD);
   575                  queue_arp_failure_cpl(ep, skb,

FAKE_CPL_PUT_EP_SAFE);

Quoted 8 lines
Quoted 8 lines
                                                  ^^^

Used after free here. These are ref counted data so it might actually be that I have misread it and miscounted the references so perhaps it's OK, but it looks might suspect at a glance.

   576          }
   577  }
Quoted 6 lines
Quoted 2 lines

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html