* [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
@ 2026-09-12 23:30 Eric Dumazet
2026-09-13 23:32 ` netdev-bot+sashiko
2026-09-16 0:10 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-09-12 23:30 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, netdev, eric.dumazet, Eric Dumazet,
Mitchell Blank Jr, Chas Williams, Qingfang Deng
In pppoatm_send(), LLC encapsulation checks whether there is sufficient
headroom for the 4-byte LLC header, but does not ensure that the skb header
is writable.
Normal transmit packets passing through ppp_start_xmit() have their header
unshared via skb_cow_head(). However, packets can also reach pppoatm_send()
via PPP channel bridging (PPPIOCBRIDGECHAN) without going through
ppp_start_xmit().
Use skb_cow_head() to ensure both sufficient headroom and a writable
header before pushing the LLC header.
While at it:
- Call pskb_may_pull(skb, 1) before inspecting skb->data[0] to prevent
out-of-bounds reads on zero-length or non-linear frames (e.g. from
bridging).
- Defer SC_COMP_PROT protocol compression until after pppoatm_may_send()
succeeds. This eliminates the temporary skb allocation on admission failure
and completely removes the fragile "undo" heuristic at the nospace label,
avoiding any risk of reading uninitialized headroom or performing an
unbalanced skb_push().
Fixes: 4cf476ced45d ("ppp: add PPPIOCBRIDGECHAN and PPPIOCUNBRIDGECHAN ioctls")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Cc: Mitchell Blank Jr <mitch@sfgoth.com>
Cc: Chas Williams <3chas3@gmail.com>
Cc: Qingfang Deng <qingfang.deng@linux.dev>
---
v2: addressed Sashiko AI review
v1: https://lore.kernel.org/netdev/20260908090519.339696-1-edumazet@google.com/
net/atm/pppoatm.c | 42 +++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 6da52d12df68e493b03f57edc46c3b5255956893..5214786e61d11aa52cf11c95c139c3a6ee471fbd 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -292,10 +292,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
struct atm_vcc *vcc;
int ret;
+ if (!pskb_may_pull(skb, 1)) {
+ kfree_skb(skb);
+ return DROP_PACKET;
+ }
+
ATM_SKB(skb)->vcc = pvcc->atmvcc;
pr_debug("(skb=0x%p, vcc=0x%p)\n", skb, pvcc->atmvcc);
- if (skb->data[0] == '\0' && (pvcc->flags & SC_COMP_PROT))
- (void) skb_pull(skb, 1);
vcc = ATM_SKB(skb)->vcc;
bh_lock_sock(sk_atm(vcc));
@@ -317,23 +320,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
switch (pvcc->encaps) { /* LLC encapsulation needed */
case e_llc:
- if (skb_headroom(skb) < LLC_LEN) {
- struct sk_buff *n;
- n = skb_realloc_headroom(skb, LLC_LEN);
- if (n != NULL &&
- !pppoatm_may_send(pvcc, n->truesize)) {
- kfree_skb(n);
- goto nospace;
- }
- consume_skb(skb);
- skb = n;
- if (skb == NULL) {
- bh_unlock_sock(sk_atm(vcc));
- return DROP_PACKET;
- }
- } else if (!pppoatm_may_send(pvcc, skb->truesize))
+ if (skb_cow_head(skb, LLC_LEN)) {
+ bh_unlock_sock(sk_atm(vcc));
+ kfree_skb(skb);
+ return DROP_PACKET;
+ }
+ if (!pppoatm_may_send(pvcc, skb->truesize))
goto nospace;
- memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
break;
case e_vc:
if (!pppoatm_may_send(pvcc, skb->truesize))
@@ -346,6 +339,12 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
return 1;
}
+ if (skb->data[0] == '\0' && (pvcc->flags & SC_COMP_PROT))
+ skb_pull(skb, 1);
+
+ if (pvcc->encaps == e_llc)
+ memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
+
atm_account_tx(vcc, skb);
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n",
skb, ATM_SKB(skb)->vcc, ATM_SKB(skb)->vcc->dev);
@@ -355,13 +354,6 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
return ret;
nospace:
bh_unlock_sock(sk_atm(vcc));
- /*
- * We don't have space to send this SKB now, but we might have
- * already applied SC_COMP_PROT compression, so may need to undo
- */
- if ((pvcc->flags & SC_COMP_PROT) && skb_headroom(skb) > 0 &&
- skb->data[-1] == '\0')
- (void) skb_push(skb, 1);
return 0;
}
--
2.55.0.1007.g17ff1f9808-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
2026-09-12 23:30 [PATCH v2 net] pppoatm: ensure a writable skb header and linear data Eric Dumazet
@ 2026-09-13 23:32 ` netdev-bot+sashiko
2026-09-13 23:49 ` Eric Dumazet
2026-09-16 0:10 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-13 23:32 UTC (permalink / raw)
To: edumazet
Cc: davem, kuba, pabeni, horms, netdev, eric.dumazet, mitch, 3chas3,
qingfang.deng
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)
- [High] pppoatm_send() does not make the frame linear, although the
commit subject claims to "ensure a writable skb header and linear…
Pre-existing issues:
- [Low] net/atm/pppoatm.c never initialises pvcc->chan.hdrlen although
the channel requires LLC_LEN (4) bytes of headroom for e_llc…
--
pppoatm: ensure a writable skb header and linear data
pppoatm_send() now uses skb_cow_head() to get a writable header with room
for the LLC header, adds a pskb_may_pull(skb, 1) before looking at
skb->data[0], and defers the SC_COMP_PROT pull plus the LLC push until
after pppoatm_may_send() has succeeded, dropping the old "undo" heuristic.
A couple of questions about the linearity claim and about the channel
headroom hint.
> diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
> index 6da52d12df68e..5214786e61d11 100644
> --- a/net/atm/pppoatm.c
> +++ b/net/atm/pppoatm.c
> @@ -292,10 +292,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
> struct atm_vcc *vcc;
> int ret;
>
> + if (!pskb_may_pull(skb, 1)) {
> + kfree_skb(skb);
> + return DROP_PACKET;
> + }
> +
[Severity: High]
The subject says "linear data" and the changelog says this pull is there
"to prevent out-of-bounds reads on zero-length or non-linear frames
(e.g. from bridging)", but does this pull actually make the frame linear?
pskb_may_pull(skb, 1) only guarantees the single byte read as
skb->data[0]; skb_cow_head() -> __skb_cow() -> pskb_expand_head()
unshares and grows the head but leaves the paged fragments in place.
So a frame with skb->len > skb_headlen(skb) still reaches the device
send op further down this function:
ret = ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
? DROP_PACKET : 1;
vcc->send is the device op (net/atm/raw.c does
vcc->send = vcc->dev->ops->send), and nothing under drivers/atm/
references nr_frags, skb_headlen or skb_linearize. For example
solos-pci psend() takes pktlen = skb->len and fpga_tx() then does:
memcpy_toio(TX_BUF(card, port), skb->data, skb->len);
or, in the DMA case:
dma_map_single(..., data, skb->len, DMA_TO_DEVICE);
For a paged skb, can this read past the end of the linear head buffer
and put unrelated heap bytes on the wire (and map a length larger than
the mapped object)?
The bridging path the changelog cites is the one that can deliver such a
frame: ppp_input() calls ppp_channel_bridge_input() before any
normalization, and the bridge hands the received skb to the peer channel
verbatim:
drivers/net/ppp/ppp_generic.c:ppp_channel_bridge_input() {
...
skb_scrub_packet(skb, !net_eq(pch->chan_net, pchb->chan_net));
if (!pchb->chan->ops->start_xmit(pchb->chan, skb))
...
}
The source channels do not linearize either (pppoe_rcv() only does
pskb_may_pull(skb, sizeof(struct pppoe_hdr)) before ppp_input(), and
l2tp_ppp.c is similar), while the normal ppp_start_xmit() path is linear
because the ppp netdev has no SG offload. Would skb_linearize() (or
pskb_may_pull(skb, skb->len)) here be the intended fix, or should the
subject and changelog be reworded so that backporters do not conclude
non-linear bridged frames are now handled?
> @@ -317,23 +320,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
>
> switch (pvcc->encaps) { /* LLC encapsulation needed */
> case e_llc:
> - if (skb_headroom(skb) < LLC_LEN) {
> - struct sk_buff *n;
> - n = skb_realloc_headroom(skb, LLC_LEN);
> - if (n != NULL &&
> - !pppoatm_may_send(pvcc, n->truesize)) {
> - kfree_skb(n);
> - goto nospace;
> - }
> - consume_skb(skb);
> - skb = n;
> - if (skb == NULL) {
> - bh_unlock_sock(sk_atm(vcc));
> - return DROP_PACKET;
> - }
> - } else if (!pppoatm_may_send(pvcc, skb->truesize))
> + if (skb_cow_head(skb, LLC_LEN)) {
> + bh_unlock_sock(sk_atm(vcc));
> + kfree_skb(skb);
> + return DROP_PACKET;
> + }
> + if (!pppoatm_may_send(pvcc, skb->truesize))
> goto nospace;
> - memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
> break;
[Severity: Low]
This isn't a bug introduced by this patch, since the previous code did the
same reallocation via skb_realloc_headroom() and dropped on failure, but
would it be worth setting pvcc->chan.hdrlen while touching this?
pppoatm_assign_vcc() sets chan.private, chan.ops and chan.mtu, and even
subtracts LLC_LEN from the MTU, but leaves chan.hdrlen at 0 (pvcc is
kzalloc'ed):
pvcc->chan.private = pvcc;
pvcc->chan.ops = &pppoatm_ops;
pvcc->chan.mtu = atmvcc->qos.txtp.max_sdu - PPP_HDRLEN -
(be.encaps == e_vc ? 0 : LLC_LEN);
include/linux/ppp_channel.h documents the field as:
int hdrlen; /* amount of headroom channel needs */
and ppp_generic honours it, e.g. in ppp_write():
skb = alloc_skb(count + pf->hdrlen, GFP_KERNEL);
if (!skb)
goto out;
skb_reserve(skb, pf->hdrlen);
With hdrlen left at 0, frames written directly to the channel fd and
multilink fragments arrive with no headroom, so the new
skb_cow_head(skb, LLC_LEN) has to run a full pskb_expand_head() with
GFP_ATOMIC inside the bh_lock_sock() section for every such frame, and
drops the packet if that allocation fails. Setting
pvcc->chan.hdrlen = LLC_LEN for the non-e_vc encapsulations would make
the common case a no-op.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260912233048.3977192-1-edumazet%40google.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
2026-09-13 23:32 ` netdev-bot+sashiko
@ 2026-09-13 23:49 ` Eric Dumazet
2026-09-13 23:58 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-09-13 23:49 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: davem, kuba, pabeni, horms, netdev, eric.dumazet, mitch, 3chas3,
qingfang.deng
On Sun, Sep 13, 2026 at 4:32 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)
>
> - [High] pppoatm_send() does not make the frame linear, although the
> commit subject claims to "ensure a writable skb header and linear…
>
> Pre-existing issues:
> - [Low] net/atm/pppoatm.c never initialises pvcc->chan.hdrlen although
> the channel requires LLC_LEN (4) bytes of headroom for e_llc…
>
> --
>
> pppoatm: ensure a writable skb header and linear data
>
> pppoatm_send() now uses skb_cow_head() to get a writable header with room
> for the LLC header, adds a pskb_may_pull(skb, 1) before looking at
> skb->data[0], and defers the SC_COMP_PROT pull plus the LLC push until
> after pppoatm_may_send() has succeeded, dropping the old "undo" heuristic.
>
> A couple of questions about the linearity claim and about the channel
> headroom hint.
>
> > diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
> > index 6da52d12df68e..5214786e61d11 100644
> > --- a/net/atm/pppoatm.c
> > +++ b/net/atm/pppoatm.c
> > @@ -292,10 +292,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
> > struct atm_vcc *vcc;
> > int ret;
> >
> > + if (!pskb_may_pull(skb, 1)) {
> > + kfree_skb(skb);
> > + return DROP_PACKET;
> > + }
> > +
>
> [Severity: High]
> The subject says "linear data" and the changelog says this pull is there
> "to prevent out-of-bounds reads on zero-length or non-linear frames
> (e.g. from bridging)", but does this pull actually make the frame linear?
> pskb_may_pull(skb, 1) only guarantees the single byte read as
> skb->data[0]; skb_cow_head() -> __skb_cow() -> pskb_expand_head()
> unshares and grows the head but leaves the paged fragments in place.
>
Linear' as in : make sure the first byte is in skb->head, before
reading skb->data[0]
> So a frame with skb->len > skb_headlen(skb) still reaches the device
> send op further down this function:
>
> ret = ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
> ? DROP_PACKET : 1;
>
> vcc->send is the device op (net/atm/raw.c does
> vcc->send = vcc->dev->ops->send), and nothing under drivers/atm/
> references nr_frags, skb_headlen or skb_linearize. For example
> solos-pci psend() takes pktlen = skb->len and fpga_tx() then does:
I would argue this is a different bug.
>
> memcpy_toio(TX_BUF(card, port), skb->data, skb->len);
>
> or, in the DMA case:
>
> dma_map_single(..., data, skb->len, DMA_TO_DEVICE);
>
> For a paged skb, can this read past the end of the linear head buffer
> and put unrelated heap bytes on the wire (and map a length larger than
> the mapped object)?
>
> The bridging path the changelog cites is the one that can deliver such a
> frame: ppp_input() calls ppp_channel_bridge_input() before any
> normalization, and the bridge hands the received skb to the peer channel
> verbatim:
>
> drivers/net/ppp/ppp_generic.c:ppp_channel_bridge_input() {
> ...
> skb_scrub_packet(skb, !net_eq(pch->chan_net, pchb->chan_net));
> if (!pchb->chan->ops->start_xmit(pchb->chan, skb))
> ...
> }
>
> The source channels do not linearize either (pppoe_rcv() only does
> pskb_may_pull(skb, sizeof(struct pppoe_hdr)) before ppp_input(), and
> l2tp_ppp.c is similar), while the normal ppp_start_xmit() path is linear
> because the ppp netdev has no SG offload. Would skb_linearize() (or
> pskb_may_pull(skb, skb->len)) here be the intended fix, or should the
> subject and changelog be reworded so that backporters do not conclude
> non-linear bridged frames are now handled?
>
> > @@ -317,23 +320,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
> >
> > switch (pvcc->encaps) { /* LLC encapsulation needed */
> > case e_llc:
> > - if (skb_headroom(skb) < LLC_LEN) {
> > - struct sk_buff *n;
> > - n = skb_realloc_headroom(skb, LLC_LEN);
> > - if (n != NULL &&
> > - !pppoatm_may_send(pvcc, n->truesize)) {
> > - kfree_skb(n);
> > - goto nospace;
> > - }
> > - consume_skb(skb);
> > - skb = n;
> > - if (skb == NULL) {
> > - bh_unlock_sock(sk_atm(vcc));
> > - return DROP_PACKET;
> > - }
> > - } else if (!pppoatm_may_send(pvcc, skb->truesize))
> > + if (skb_cow_head(skb, LLC_LEN)) {
> > + bh_unlock_sock(sk_atm(vcc));
> > + kfree_skb(skb);
> > + return DROP_PACKET;
> > + }
> > + if (!pppoatm_may_send(pvcc, skb->truesize))
> > goto nospace;
> > - memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
> > break;
>
> [Severity: Low]
> This isn't a bug introduced by this patch, since the previous code did the
> same reallocation via skb_realloc_headroom() and dropped on failure, but
> would it be worth setting pvcc->chan.hdrlen while touching this?
>
> pppoatm_assign_vcc() sets chan.private, chan.ops and chan.mtu, and even
> subtracts LLC_LEN from the MTU, but leaves chan.hdrlen at 0 (pvcc is
> kzalloc'ed):
>
> pvcc->chan.private = pvcc;
> pvcc->chan.ops = &pppoatm_ops;
> pvcc->chan.mtu = atmvcc->qos.txtp.max_sdu - PPP_HDRLEN -
> (be.encaps == e_vc ? 0 : LLC_LEN);
>
> include/linux/ppp_channel.h documents the field as:
>
> int hdrlen; /* amount of headroom channel needs */
>
> and ppp_generic honours it, e.g. in ppp_write():
>
> skb = alloc_skb(count + pf->hdrlen, GFP_KERNEL);
> if (!skb)
> goto out;
> skb_reserve(skb, pf->hdrlen);
>
> With hdrlen left at 0, frames written directly to the channel fd and
> multilink fragments arrive with no headroom, so the new
> skb_cow_head(skb, LLC_LEN) has to run a full pskb_expand_head() with
> GFP_ATOMIC inside the bh_lock_sock() section for every such frame, and
> drops the packet if that allocation fails. Setting
> pvcc->chan.hdrlen = LLC_LEN for the non-e_vc encapsulations would make
> the common case a no-op.
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260912233048.3977192-1-edumazet%40google.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
2026-09-13 23:49 ` Eric Dumazet
@ 2026-09-13 23:58 ` Eric Dumazet
2026-09-14 10:33 ` Simon Horman
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-09-13 23:58 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: davem, kuba, pabeni, horms, netdev, eric.dumazet, mitch, 3chas3,
qingfang.deng
On Sun, Sep 13, 2026 at 4:49 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sun, Sep 13, 2026 at 4:32 PM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)
> >
> > - [High] pppoatm_send() does not make the frame linear, although the
> > commit subject claims to "ensure a writable skb header and linear…
> >
> > Pre-existing issues:
> > - [Low] net/atm/pppoatm.c never initialises pvcc->chan.hdrlen although
> > the channel requires LLC_LEN (4) bytes of headroom for e_llc…
> >
> > --
> >
> > pppoatm: ensure a writable skb header and linear data
> >
> > pppoatm_send() now uses skb_cow_head() to get a writable header with room
> > for the LLC header, adds a pskb_may_pull(skb, 1) before looking at
> > skb->data[0], and defers the SC_COMP_PROT pull plus the LLC push until
> > after pppoatm_may_send() has succeeded, dropping the old "undo" heuristic.
> >
> > A couple of questions about the linearity claim and about the channel
> > headroom hint.
> >
> > > diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
> > > index 6da52d12df68e..5214786e61d11 100644
> > > --- a/net/atm/pppoatm.c
> > > +++ b/net/atm/pppoatm.c
> > > @@ -292,10 +292,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
> > > struct atm_vcc *vcc;
> > > int ret;
> > >
> > > + if (!pskb_may_pull(skb, 1)) {
> > > + kfree_skb(skb);
> > > + return DROP_PACKET;
> > > + }
> > > +
> >
> > [Severity: High]
> > The subject says "linear data" and the changelog says this pull is there
> > "to prevent out-of-bounds reads on zero-length or non-linear frames
> > (e.g. from bridging)", but does this pull actually make the frame linear?
> > pskb_may_pull(skb, 1) only guarantees the single byte read as
> > skb->data[0]; skb_cow_head() -> __skb_cow() -> pskb_expand_head()
> > unshares and grows the head but leaves the paged fragments in place.
> >
>
>
> Linear' as in : make sure the first byte is in skb->head, before
> reading skb->data[0]
I guess I need to invent a new term : 1-byte-linear
Perhaps this will please/silence our AI friends.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
2026-09-13 23:58 ` Eric Dumazet
@ 2026-09-14 10:33 ` Simon Horman
0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2026-09-14 10:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-bot+sashiko, davem, kuba, pabeni, netdev, eric.dumazet,
mitch, 3chas3, qingfang.deng
On Sun, Sep 13, 2026 at 04:58:47PM -0700, Eric Dumazet wrote:
> On Sun, Sep 13, 2026 at 4:49 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Sun, Sep 13, 2026 at 4:32 PM <netdev-bot+sashiko@kernel.org> wrote:
> > >
> > > Thank you for your contribution! Sashiko AI review found 1 potential
> > > issue(s) to consider:
> > >
> > > Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1)
> > >
> > > - [High] pppoatm_send() does not make the frame linear, although the
> > > commit subject claims to "ensure a writable skb header and linear…
> > >
> > > Pre-existing issues:
> > > - [Low] net/atm/pppoatm.c never initialises pvcc->chan.hdrlen although
> > > the channel requires LLC_LEN (4) bytes of headroom for e_llc…
> > >
> > > --
> > >
> > > pppoatm: ensure a writable skb header and linear data
> > >
> > > pppoatm_send() now uses skb_cow_head() to get a writable header with room
> > > for the LLC header, adds a pskb_may_pull(skb, 1) before looking at
> > > skb->data[0], and defers the SC_COMP_PROT pull plus the LLC push until
> > > after pppoatm_may_send() has succeeded, dropping the old "undo" heuristic.
> > >
> > > A couple of questions about the linearity claim and about the channel
> > > headroom hint.
> > >
> > > > diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
> > > > index 6da52d12df68e..5214786e61d11 100644
> > > > --- a/net/atm/pppoatm.c
> > > > +++ b/net/atm/pppoatm.c
> > > > @@ -292,10 +292,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
> > > > struct atm_vcc *vcc;
> > > > int ret;
> > > >
> > > > + if (!pskb_may_pull(skb, 1)) {
> > > > + kfree_skb(skb);
> > > > + return DROP_PACKET;
> > > > + }
> > > > +
> > >
> > > [Severity: High]
> > > The subject says "linear data" and the changelog says this pull is there
> > > "to prevent out-of-bounds reads on zero-length or non-linear frames
> > > (e.g. from bridging)", but does this pull actually make the frame linear?
> > > pskb_may_pull(skb, 1) only guarantees the single byte read as
> > > skb->data[0]; skb_cow_head() -> __skb_cow() -> pskb_expand_head()
> > > unshares and grows the head but leaves the paged fragments in place.
> > >
> >
> >
> > Linear' as in : make sure the first byte is in skb->head, before
> > reading skb->data[0]
>
> I guess I need to invent a new term : 1-byte-linear
>
> Perhaps this will please/silence our AI friends.
It's the brave new world that we live in.
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data
2026-09-12 23:30 [PATCH v2 net] pppoatm: ensure a writable skb header and linear data Eric Dumazet
2026-09-13 23:32 ` netdev-bot+sashiko
@ 2026-09-16 0:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-16 0:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, horms, netdev, eric.dumazet, mitch, 3chas3,
qingfang.deng
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 12 Sep 2026 23:30:48 +0000 you wrote:
> In pppoatm_send(), LLC encapsulation checks whether there is sufficient
> headroom for the 4-byte LLC header, but does not ensure that the skb header
> is writable.
>
> Normal transmit packets passing through ppp_start_xmit() have their header
> unshared via skb_cow_head(). However, packets can also reach pppoatm_send()
> via PPP channel bridging (PPPIOCBRIDGECHAN) without going through
> ppp_start_xmit().
>
> [...]
Here is the summary with links:
- [v2,net] pppoatm: ensure a writable skb header and linear data
https://git.kernel.org/netdev/net/c/ecc7253683a3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-16 0:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-12 23:30 [PATCH v2 net] pppoatm: ensure a writable skb header and linear data Eric Dumazet
2026-09-13 23:32 ` netdev-bot+sashiko
2026-09-13 23:49 ` Eric Dumazet
2026-09-13 23:58 ` Eric Dumazet
2026-09-14 10:33 ` Simon Horman
2026-09-16 0:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox