Re: [PATCH v2 net] pppoatm: ensure a writable skb header and linear data

[Simon Horman <horms@kernel.org>]

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>