[ofa-general] Re: Dereferencing freed memory bugs

[Latif, Faisal]

Thanks Dan for the input. There were 3 issues (comments inline) that required changes while others were OK. I will be creating a patch for them as you pointed out.

Faisal

>-----Original Message-----
>From: Dan Carpenter [mailto:error27 at gmail.com]
>Sent: Thursday, April 02, 2009 1:38 AM
>To: Tung, Chien Tin
>Cc: Roland Dreier; Marcin Slusarz; general at lists.openfabrics.org
>Subject: Re: [ofa-general] Re: Dereferencing freed memory bugs
>
>I checked it with 2.6.29-git9.  There were still a couple issues in
>drivers/infiniband/hw/nes/nes_cm.c.
>
>   428                  if (cm_node->recv_entry) {
>   429                          WARN_ON(1);
>   430                          return -EINVAL;
>   431                  }
>
>missing kfree(new_send);

Will be adding the kfree() before the WARN_ON(.

>
>   521                  rem_ref_cm_node(cm_node->cm_core, cm_node);
>   522          }
>   523          if (cm_node->cm_id)
>   524                  cm_id->rem_ref(cm_id);
>
>dereferencing freed memory.  rem_ref_cm_node can call kfree on cm_node.
>
>   662
>rem_ref_cm_node(cm_node->cm_core,
>   663                                                  cm_node);
>   664                          }
>   665                  } while (0);
>   666
>   667
>spin_unlock_irqrestore(&cm_node->retrans_list_lock, flags);
>
>same.

All the above were OK in the code. rem_ref_cm_node() will not be freeing up the cm_node memory as its ref_count will always be greater than 1 in above situations. The rem_ref_cm_node() will be decrementing the ref_count and returning.

>
>  1265          cm_node->freed = 1;
>  1266          kfree(cm_node);
>
>You can't actually checked cm_node->freed if it's freed.

Yes, this was leftover from previous debugging where we had commented out kfree() temporarily to find a bug. I removed the freed flag from the cm_node as it was not used anywhere else.


>
>  2007                          loopbackremotenode =
>make_cm_node(cm_core, nesvnic,
>  2008                                  &loopback_cm_info,
>loopbackremotelistener);
>  2009                          loopbackremotenode->loopbackpartner =
>cm_node;
>
>make_cm_node() returns NULL in low memory situations.

Yes I will fix the above.

>
>Don't forget to add the reported by sticker.  :P
>Reported-by: Dan Carpenter <error27 at gmail.com>
>
>regards,
>dan carpenter