Re: [lwip-devel] Problem with LWIP_ERROR in pbuf_header(..)

[Fredrik Hederstierna]

Kieran Mansley <address@hidden> wrote on
04/24/2009 04:56:08 PM:

> On Fri, 2009-04-24 at 14:36 +0200, Fredrik Hederstierna wrote:
> > 
> > Hi 
> > 
> > We have problems with getting LWIP_ERROR() in function  pbuf_header
> > (..) in "pbuf.c" 
> > 
> > 
> > The ASSERTING pbuf.c code looks like this, line 429 
> > 
> > 
> >   if (header_size_increment < 0){ 
> >     increment_magnitude = -header_size_increment; 
> >          /* Check that we aren't going
to move off the end of the pbuf
> > */ 
> > -------->LWIP_ERROR("increment_magnitude <= p->len",
> > (increment_magnitude <= p->len), return 1;); 
> > 
> > 
> > 
> > 
> > We suspect that its called from UDP code in udp_input(..) line
102: 
> > 
> > 
> >   /* Check minimum length (IP header + UDP header) 
> >    * and move payload pointer to UDP header */ 
> >   if (p->tot_len < (IPH_HL(iphdr) * 4 + UDP_HLEN)
|| pbuf_header(p, -
> > (s16_t)(IPH_HL(iphdr) * 4))) { 
> > 
> > 
> > That is if incomming UDP packet is too short, so the call to
> > pbuf_header() will trig LWIP_ERROR? 
> > Isn't it better to just drop illegal-size packets instead of
ASSERT? 
> > What if header and header length is corrupted? 
> 
> In the case you mention, shouldn't the first clause in the if statement
> "if (p->tot_len < (IPH_HL(iphdr) * 4 + UDP_HLEN)"
 protect us from the
> assertion?  I.e. if the packet is short it should do that clause,
but
> then not the pbuf_header() call.


But is it true that always  (p->tot_len ==
p->len)  here ?
Maybe due to almost full memory  p->tot_len
 is  larger than  p->len ?
The ERROR is checking  p->len,  but if-statement
checks  p->tot_len.
Are we always in last packet here, what if we got
a queue with more pbufs?
(I'm a quite new lwIP user, so maybe I've misunderstood
something.)

> Also, it's a good idea subscribe to lwip-devel to make sure you see
> replies to your questions.  I've CC'd you as I noticed in this
case that
> you're not subscribed, but I don't always realise!

Yes, you are right, I should sign up for the mailing
list, thanks for notifying me!

Thanks/Fredrik