Hi, I didn't analyse this bug report but probably it is nearly connected with one of the bugs visible in a log from this submit:
http://bugzilla.kernel.org/show_bug.cgi?id=8132 On 15-04-2007 02:50, Paul Mackerras wrote: > David Miller writes: > >> Here is Patrick McHardy's patch: > > So this doesn't change process_input_packet(), which treats the case > where the first byte is 0xff (PPP_ALLSTATIONS) but the second byte is > 0x03 (PPP_UI) as indicating a packet with a PPP protocol number of > 0xff. Arguably that's wrong since PPP protocol 0xff is reserved, and > the RFC does envision the possibility of receiving frames where the > control field has values other than 0x03. > > Therefore I think this patch is probably better. Could people try it > out and let me know if it fixes the problem? > > Paul. > > diff --git a/drivers/net/ppp_async.c b/drivers/net/ppp_async.c > index 933e2f3..caabbc4 100644 > --- a/drivers/net/ppp_async.c > +++ b/drivers/net/ppp_async.c > @@ -802,9 +802,9 @@ process_input_packet(struct asyncppp *ap) > > /* check for address/control and protocol compression */ > p = skb->data; > - if (p[0] == PPP_ALLSTATIONS && p[1] == PPP_UI) { > + if (p[0] == PPP_ALLSTATIONS) { > /* chop off address/control */ > - if (skb->len < 3) > + if (p[1] != PPP_UI || skb->len < 3) > goto err; > p = skb_pull(skb, 2); > } Let's look farther: > proto = p[0]; > if (proto & 1) { > /* protocol is compressed */ > skb_push(skb, 1)[0] = 0; BTW - about Patrick's patch: skb_push seems to be dependent here on the 1-st char of skb->data, if above (p[0] != PPP_ALLSTATIONS), but on the 3-rd char otherwise (after skb_pull). But, Patrick's patch reserves the place for this, looking always at 1-st char (buf[0]) independently of PPP_ALLSTATIONS char presence, or otherwise - always treating this char as protocol char. It looks safe because of PPP_ALLSTATION current value, but isn't too understandable. On the other hand, without any reservation in the ppp_async_input for the (buf[0] == PPP_ALLSTATIONS) case, probably 4-byte alignement isn't achieved as planned. > } else { > if (skb->len < 2) > goto err; > proto = (proto << 8) + p[1]; > if (proto == PPP_LCP) > async_lcp_peek(ap, p, skb->len, 1); > } > > /* queue the frame to be processed */ > skb->cb[0] = ap->state; > skb_queue_tail(&ap->rqueue, skb); > ap->rpkt = NULL; > ap->state = 0; > return; > > err: > /* frame had an error, remember that, reset SC_TOSS & SC_ESCAPE */ > ap->state = SC_PREV_ERROR; > if (skb) { > /* make skb appear as freshly allocated */ Probably this isn't always true and here the problem started... > skb_trim(skb, 0); > skb_reserve(skb, - skb_headroom(skb)); Isn't here lost e.g. NET_SKB_PAD probably reserved by dev_alloc_skb? On the other hand - this kind of pad can very good hide similar reservation problems in many other places - maybe it should be omitted or somehow counted in WARNs when some debugging options are active? Regards, Jarek P. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
