2001-11-13 21:21:15

by Till Immanuel Patzschke

[permalink] [raw]
Subject: [PATCH] ppp_generic causes skput:under: w/ pppoatm and vc-encaps

diff -Naur linux-2.4.10/drivers/net/ppp_generic.c linux-2.4.10-new/drivers/net/ppp_generic.c
--- linux-2.4.10/drivers/net/ppp_generic.c Sun Sep 9 19:45:43 2001
+++ linux-2.4.10-new/drivers/net/ppp_generic.c Tue Nov 13 21:48:29 2001
@@ -1470,6 +1470,23 @@
/* check if the packet passes the pass and active filters */
/* the filter instructions are constructed assuming
a four-byte PPP header on each packet */
+ /* make sure we have room for the following skb_push... */
+ if (skb_headroom(skb) < PPP_HDRLEN) {
+ struct sk_buff *ns;
+
+ ns = alloc_skb(skb->len + PPP_HDRLEN*2,GFP_ATOMIC);
+ if (ns == 0) {
+ printk(KERN_DEBUG "PPP: inbound skb not resizeable.\n");
+ kfree_skb(skb);
+ return;
+ }
+ skb_reserve(ns, PPP_HDRLEN*2);
+ memcpy(skb_put(ns, skb->len), skb->data, skb->len);
+ kfree_skb(skb);
+ skb = ns;
+ if (ppp->debug & 1)
+ printk(KERN_DEBUG "PPP: inbound skb resized.\n");
+ }
*skb_push(skb, 2) = 0;
if (ppp->pass_filter.filter
&& sk_run_filter(skb, ppp->pass_filter.filter,


Attachments:
qq2 (1.07 kB)

2001-11-14 05:02:40

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm and vc-encaps

From: Till Immanuel Patzschke <[email protected]>
Date: Tue, 13 Nov 2001 22:30:24 +0100

I've attached a patch, checking for headroom first, and - if necessary -
reallocating a larger buffer for the skb_push.

Please check and apply - or find a better fix!

Here is my "better fix". In pppoatm, we should be increasing the
device header length appropriately. ie. dev->hard_header_len needs to
be increased in the pppoatm driver when vc-encaps is used.

Franks a lot,
David S. Miller
[email protected]

2001-11-14 11:03:58

by Till Immanuel Patzschke

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm andvc-encaps

"David S. Miller" wrote:

> From: Till Immanuel Patzschke <[email protected]>
> Date: Tue, 13 Nov 2001 22:30:24 +0100
>
> I've attached a patch, checking for headroom first, and - if necessary -
> reallocating a larger buffer for the skb_push.
>
> Please check and apply - or find a better fix!
>
> Here is my "better fix". In pppoatm, we should be increasing the
> device header length appropriately. ie. dev->hard_header_len needs to
> be increased in the pppoatm driver when vc-encaps is used.

that is - of course - a more pppoatm specific fix BUT my concern ist that
PPP requires (!) to have at least 2 bytes headroom [when using PPP_FILTER]
(which is NOT noted), and isn't it good practice to check for room before
claiming it?


--
Till Immanuel Patzschke mailto: [email protected]
interNetwork AG Phone: +49-(0)611-1731-121
Bierstadter Str. 7 Fax: +49-(0)611-1731-31
D-65189 Wiesbaden Web: http://www.internetwork-ag.de



2001-11-14 11:48:42

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm andvc-encaps

From: Till Immanuel Patzschke <[email protected]>
Date: Wed, 14 Nov 2001 12:13:10 +0100

that is - of course - a more pppoatm specific fix BUT my concern
ist that PPP requires (!) to have at least 2 bytes headroom [when
using PPP_FILTER] (which is NOT noted), and isn't it good practice
to check for room before claiming it?

Ignore my comments, I thought this was happening on output.
Yes, it seems ppp_generic should be verifying the headroom.

Franks a lot,
David S. Miller
[email protected]

2001-11-14 14:24:16

by Michal Ostrowski

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm and vc-encaps

If you set the hdrlen field of the ppp_channel that pppoatm registers
(pvcc->chan.hdrlen, in pppoatm_assign_vcc()) then ppp_generic will
always over-allocate skb space to allow for extra headers to be pushed
in. This mechanism was put is so that we wouldn't have to copy the
frame in order to slap on PPPoE headers onto it. I think it's a good
idea to be doing this, especially if you're going to play with
hard_header_len.

If you look at pppoatm_send(), you'll see that you do an
skb_realloc_headroom if there's no space for the headers. If
pvcc->chan.hdrlen is set properly then this will be the exceptional,
rather than the common case.



> Here is my "better fix". In pppoatm, we should be increasing the
> device header length appropriately. ie. dev->hard_header_len needs to
> be increased in the pppoatm driver when vc-encaps is used.
>
> Franks a lot,
> David S. Miller
> [email protected]


--
Michal Ostrowski
[email protected]

2001-11-14 14:29:06

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm and vc-encaps

From: Michal Ostrowski <[email protected]>
Date: 14 Nov 2001 09:23:53 -0500

If you look at pppoatm_send(), you'll see that you do an
skb_realloc_headroom if there's no space for the headers. If
pvcc->chan.hdrlen is set properly then this will be the exceptional,
rather than the common case.

Their problem is on receive, not send. That's problematic because in
that case the ATM drivers allocate and size the SKBs.

2001-11-14 14:32:46

by Till Immanuel Patzschke

[permalink] [raw]
Subject: Re: [PATCH] ppp_generic causes skput:under: w/ pppoatm and vc-encaps

Hi Michal,

it is the receive side - (chan.hdrlen is set correctly) -- on the receive side
you'll the SKB from the device AND headroom is up to the device...
Michal Ostrowski wrote:

> If you set the hdrlen field of the ppp_channel that pppoatm registers
> (pvcc->chan.hdrlen, in pppoatm_assign_vcc()) then ppp_generic will
> always over-allocate skb space to allow for extra headers to be pushed
> in. This mechanism was put is so that we wouldn't have to copy the
> frame in order to slap on PPPoE headers onto it. I think it's a good
> idea to be doing this, especially if you're going to play with
> hard_header_len.
>
> If you look at pppoatm_send(), you'll see that you do an
> skb_realloc_headroom if there's no space for the headers. If
> pvcc->chan.hdrlen is set properly then this will be the exceptional,
> rather than the common case.
>
> > Here is my "better fix". In pppoatm, we should be increasing the
> > device header length appropriately. ie. dev->hard_header_len needs to
> > be increased in the pppoatm driver when vc-encaps is used.
> >
> > Franks a lot,
> > David S. Miller
> > [email protected]
>
> --
> Michal Ostrowski
> [email protected]

--
Till Immanuel Patzschke mailto: [email protected]
interNetwork AG Phone: +49-(0)611-1731-121
Bierstadter Str. 7 Fax: +49-(0)611-1731-31
D-65189 Wiesbaden Web: http://www.internetwork-ag.de