Re: [PATCH] ppp: make room for the filter tag instead of assuming it

On Wed, Sep 2, 2026 at 8:22 AM Vlatko Kosturjak
[off-list ref] wrote:

ppp_receive_nonmp_frame() prepends a two-byte direction tag before running
the pass/active BPF filters:

        *(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_INBOUND_TAG);

Nothing on the receive path guarantees those two bytes of headroom, and a
peer can arrange for only one to be available. ppp_async.c contributes two
halves of the problem.

First, the frame-error path resets a reused skb's headroom to zero while
claiming to restore it to a freshly allocated state - but a fresh skb from
dev_alloc_skb() carries NET_SKB_PAD:

        err:
                if (skb) {
                        /* make skb appear as freshly allocated */
                        skb_trim(skb, 0);
                        skb_reserve(skb, - skb_headroom(skb));
                }

Thanks for the patch and the reproducer.

The analysis of the panic is correct, but instead of working around
this root cause,
I think we should drop the packet (like ppp_synctty does).

Resetting headroom to 0 completely violates the NET_SKB_PAD guarantee
provided by dev_alloc_skb().

Not only does this lead to the filter panic you observed, but if CCP
compression is enabled, ppp_decompress_frame() passes `skb->data - 2`
to decompress()/incomp(), which reads out of bounds before skb->head.

Use in V2 : Fixes:1da177e4c3f4 ("Linux-2.6.12-rc2")

Also please target [PATCH net] in the subject prefix.

Please wait ~24 hours before sending a V2.

Thanks!

I am guessing the skb_reseve() could be replaced with skb_reserve(skb,
NET_SKB_PAD - skb_headroom(skb));

But IMO a drop would be better.

diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
index ea7fe9608ffd6580f584afd3c61cdd29e67474d3..6e6e2b9441285d6e16ee7610bd59344b9e3d3e52

100644

--- a/drivers/net/ppp/ppp_async.c
+++ b/drivers/net/ppp/ppp_async.c

@@ -742,11 +742,8 @@ process_input_packet(struct asyncppp *ap)
  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 */
-               skb_trim(skb, 0);
-               skb_reserve(skb, - skb_headroom(skb));
-       }
+       kfree_skb(skb);
+       ap->rpkt = NULL;
 }

 /* Called when the tty driver has data for us. Runs parallel with the