buglwIP - A Lightweight TCP/IP stack - Bugs: bug #19729, pbuf_free problem

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #19729: pbuf_free problem

Submitted by:  Cui hengbin <phant>
Submitted on:  Sat 28 Apr 2007 08:28:21 AM UTC  
 
Category: pbufsSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: None

(Jump to the original submission Jump to the original submission)

Thu 17 May 2007 09:06:57 AM UTC, comment #9:

You're right. Done that. Reading the code I ask myself why ip_input has a return type of err_t instead of void since it always returns ERR_OK and frees the pbuf itself.

OK, but now I'll really close it as fixed.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 16 May 2007 08:59:04 PM UTC, comment #8:

So, if current ethernetif.c is good, you should remove pbuf_free(p) from tcpip_input & tcpip_ethinput in tcpip.c, when "msg = memp_malloc(MEMP_TCPIP_MSG);" failed, like you have propose in comment #2, no ?

(be careful to tabs)

Frédéric Bernon <fbernon>
Project Member
Wed 16 May 2007 07:55:04 PM UTC, comment #7:

ethernetif.c is already up to date, checked in the others.

What remains is ppp.c: return value is not checked but pbuf is freed all the time (regardless of the return value of ip_input, which returns ERR_OK all the time, anyway. Personally, I don't think that's right, but as I don't know/understand the ppp code anyway, I think it is not really related to this bug).

Closing this as fixed.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 16 May 2007 07:01:14 PM UTC, comment #6:

Ok, no problem for me !!! Can you just also update ethernetif.c ?

Frédéric Bernon <fbernon>
Project Member
Wed 16 May 2007 06:58:54 PM UTC, comment #5:

Frédéric, can I check in loopif.c, ppp.c & slipif.c that check netif->input() return value and delete p if != NULL?

It's assigned to you, but I already have it changed locally, so...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 30 Apr 2007 11:52:05 AM UTC, comment #4:

So I think we should update loopif to free the pbuf if netif->input() fails!

PPP also calls netif->input() without checking the return value. That would have to be changed. However, as long as ip_input() is used, that doesn't matter since ip_input() always returns ERR_OK and frees the pbuf itself on error.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 29 Apr 2007 09:22:41 PM UTC, comment #3:

I'm agree With Simon is we only study ethernetif use, but I'm not sure if loopif and ppp interface handle that correctly :

In loopif, no check is done, and in ppp, I don't know...

Frédéric Bernon <fbernon>
Project Member
Sun 29 Apr 2007 10:30:46 AM UTC, comment #2:

I think it would be better to change tcpip.c (tcpip_input() and tcpip_ethinput()) not to free the pbuf but simply returning != ERR_OK and letting the ethernet driver free the pbuf. I think that's more consistent behaviour.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 29 Apr 2007 03:29:30 AM UTC, comment #1:

so sorry ,in ethernetif.c should change as followed

if (netif->input(p, netif)==ERR_VAL)
{ LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_init: IP input error\n"));
pbuf_free(p);
p = NULL;
}

Cui hengbin <phant>
Sat 28 Apr 2007 08:28:21 AM UTC, original submission:

Hi ALL:
When I define ETHARP_TCPIP_ETHINPUT , using sniffer generating broadcast frame to network,there always assert at line 606 in pbuf.c .
But I find it a bug,
in tcpip.c when no memory can malloc ,free it ,but in ethernetif.c ethernetif_input free it again.

#if ETHARP_TCPIP_ETHINPUT
err_t
tcpip_ethinput(struct pbuf p, struct netif inp)
{
struct tcpip_msg *msg;

if (mbox != SYS_MBOX_NULL) {
msg = memp_malloc(MEMP_TCPIP_MSG);
if (msg == NULL) {
pbuf_free(p); //free here
return ERR_MEM;
}

msg->type = TCPIP_MSG_ETHINPUT;
msg->msg.inp.p = p;
msg->msg.inp.netif = inp;
sys_mbox_post(mbox, msg);
return ERR_OK;
}
return ERR_VAL;
}
#endif /* ETHARP_TCPIP_ETHINPUT */

static void
ethernetif_input(struct netif *netif)
{
struct ethernetif *ethernetif;
struct eth_hdr *ethhdr;
struct pbuf *p;

ethernetif = netif->state;

/* move received packet into a new pbuf */
p = low_level_input(netif);
/* no packet could be read, silently ignore this */
if (p == NULL) return;
/* points to packet payload, which starts with an Ethernet header */
ethhdr = p->payload;

switch (htons(ethhdr->type)) {

#if ETHARP_TCPIP_ETHINPUT
/* IP or ARP packet? */
case ETHTYPE_IP:
case ETHTYPE_ARP:
/* full packet send to tcpip_thread to process */
if (netif->input(p, netif)!=ERR_OK)
{ LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_init: IP input error\n"));
pbuf_free(p);
p = NULL;
}
break;

#else /* ETHARP_TCPIP_ETHINPUT */
#if ETHARP_TCPIP_INPUT

/* IP packet? */
case ETHTYPE_IP:
#if ETHARP_TRUST_IP_MAC
/* update ARP table */
etharp_ip_input(netif, p);
#endif /* ETHARP_TRUST_IP_MAC */
/* skip Ethernet header */
pbuf_header(p, -sizeof(struct eth_hdr));
/* pass to network layer */
if (netif->input(p, netif)!=ERR_OK) /* should change to "if (netif->input(p, netif)==ERR_MEM)"*/

{ LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_init: IP input error\n"));
pbuf_free(p); /* free again so cause asserting in pbuf.c*/
p = NULL;
}
break;

/* ARP packet? */
case ETHTYPE_ARP:
/* pass p to ARP module */
etharp_arp_input(netif, ethernetif->ethaddr, p);
break;

#endif /* ETHARP_TCPIP_INPUT */
#endif /* ETHARP_TCPIP_ETHINPUT */

default:
pbuf_free(p);
p = NULL;
break;
}
}

FYI:

Cuihengbin

----------------------------------------------------- 

Cui hengbin <phant>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by fbernon (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by phant (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 17 May 2007 09:06:57 AM UTCgoldsimonStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Wed 16 May 2007 07:01:14 PM UTCfbernonAssigned tofbernon=>goldsimon
    Thu 03 May 2007 10:25:54 AM UTCfbernonAssigned toNone=>fbernon

    Back to the top


    Powered by Savane 3.1-cleanup1