Thread (3 messages) flat view 3 messages, 3 authors, 2d ago
WARM2d

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

From: Vlatko Kosturjak <hidden>
Date: 2026-09-02 06:22:25
Subsystem: networking drivers, ppp protocol drivers and compressors, the rest · Maintainers: Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

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));
                }

ap->rpkt still points at that skb, so the next frame reuses it with no
headroom at all.

Second, the headroom for a new frame is chosen from a single byte, and the
comment already notes the test is incomplete:

        if (skb->len == 0) {
                /* Try to get the payload 4-byte aligned. This should match
                 * the PPP_ALLSTATIONS/PPP_UI/compressed tests in
                 * process_input_packet, but we do not have enough chars here
                 * to test buf[1] and buf[2].
                 */
                if (buf[0] != PPP_ALLSTATIONS)
                        skb_reserve(skb, 2 + (buf[0] & 1));
        }

A frame beginning 0xff reserves nothing.

So a frame with a bad FCS zeroes the reused skb's headroom; the next frame
begins ff 03, so nothing is reserved; process_input_packet() pulls the
two-byte address/control field, giving two bytes of headroom; and
__ppp_decompress_proto() pushes one back for a PFC-compressed protocol byte,
leaving one. The filter tag then pushes two and lands a byte below skb->head:

  skbuff: skb_under_panic: len:49 put:2 head:ffff888003c10000
          data:ffff888003c0ffff tail:0x30 end:0x640 dev:<NULL>
  kernel BUG at net/core/skbuff.c:214!
  RIP: 0010:skb_panic+0x13e/0x230
  Call Trace:
   skb_push+0xbd/0x100
   ppp_receive_nonmp_frame+0x48a/0x1d10
   ppp_input+0x4e9/0x2f80
   ppp_async_process+0x2a/0xe0
   tasklet_action_common+0x20f/0x8a0
   handle_softirqs+0x18e/0x590
  Kernel panic - not syncing: Fatal exception in interrupt

Two frames from the peer are enough. It needs CONFIG_PPP_FILTER and a link
with PPPIOCSPASS or PPPIOCSACTIVE applied, which is what pppd does for
demand-dialling and for active-filter. The panic happens in softirq context,
so it takes the machine down rather than the link.

Rather than tighten the headroom bookkeeping in one transport - ppp_synctty
and any other channel driver make no such guarantee either - make room at
the site that needs it. skb_cow_head() also subsumes the skb_unclone() it
replaces.

Found by fuzzing the PPP receive path with a mutating peer on a pty;
It is interesting (remote) DoS: Root configures PPP; peer supplies two crashing
frames.

I have a working reproducer in attachment - generated with help of GenAI.
It needs root only because /dev/ppp is 0600 by default - the frames
that trigger
the panic arrive from the tty side, i.e. from the peer, so on a
configured link this
is remotely reachable.

Tested on pristine v7.2.2.

Fixes: the PPP_FILTER_INBOUND_TAG push in ppp_receive_nonmp_frame()
Signed-off-by: Vlatko Kosturjak <redacted>

---
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 1111111..2222222 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -2498,7 +2498,10 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct
sk_buff *skb)

 #ifdef CONFIG_PPP_FILTER
                if (ppp->pass_filter || ppp->active_filter) {
-                       if (skb_unclone(skb, GFP_ATOMIC))
+                       /* The receive path does not guarantee 2 bytes of
+                        * headroom here, so make room rather than assume it.
+                        */
+                       if (skb_cow_head(skb, 2))
                                goto err;
                        /* Check if the packet passes the pass and
active filters.
                         * See comment for PPP_FILTER_INBOUND_TAG above.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help