[PATCH net v2] pppoe: drop PFC frames

[PATCH net v2] pppoe: drop PFC frames

[Qingfang Deng]

RFC 2516 Section 7 states that Protocol Field Compression (PFC) is NOT
RECOMMENDED for PPPoE. In practice, pppd does not support negotiating
PFC for PPPoE sessions, and the current PPPoE driver assumes an
uncompressed (2-byte) protocol field. However, the generic PPP layer
function ppp_input() is not aware of the negotiation result, and still
accepts PFC frames.

If a peer with a broken implementation or an attacker sends a frame with
a compressed (1-byte) protocol field, the subsequent PPP payload is
shifted by one byte. This causes the network header to be 4-byte
misaligned, which may trigger unaligned access exceptions on some
architectures.

To reduce the attack surface, drop PPPoE PFC frames. Introduce
ppp_skb_is_compressed_proto() helper function to be used in both
ppp_generic.c and pppoe.c to avoid open-coding.

Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers")
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
v2: retarget to net, and add a helper function
 https://lore.kernel.org/netdev/20260403083926.68320-1-qingfang.deng@linux.dev/

 drivers/net/ppp/ppp_generic.c |  2 +-
 drivers/net/ppp/pppoe.c       |  8 +++++++-
 include/linux/ppp_defs.h      | 14 ++++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index cb29a6968c63..eb1503b389ef 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -2251,7 +2251,7 @@ ppp_do_recv(struct ppp *ppp, struct sk_buff *skb, struct channel *pch)
  */
 static void __ppp_decompress_proto(struct sk_buff *skb)
 {
-	if (skb->data[0] & 0x01)
+	if (ppp_skb_is_compressed_proto(skb))
 		*(u8 *)skb_push(skb, 1) = 0x00;
 }
 
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 1ac61c273b28..4cd10c908711 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -393,7 +393,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb_mac_header_len(skb) < ETH_HLEN)
 		goto drop;
 
-	if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
+	if (!pskb_may_pull(skb, PPPOE_SES_HLEN))
 		goto drop;
 
 	ph = pppoe_hdr(skb);
@@ -403,6 +403,12 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb->len < len)
 		goto drop;
 
+	/* skb->data points to the PPP protocol header after skb_pull_rcsum.
+	 * Drop PFC frames.
+	 */
+	if (ppp_skb_is_compressed_proto(skb))
+		goto drop;
+
 	if (pskb_trim_rcsum(skb, len))
 		goto drop;
 
diff --git a/include/linux/ppp_defs.h b/include/linux/ppp_defs.h
index b74209e5a11e..ba6e53655b93 100644
--- a/include/linux/ppp_defs.h
+++ b/include/linux/ppp_defs.h
@@ -8,10 +8,24 @@
 #define _PPP_DEFS_H_
 
 #include <linux/crc-ccitt.h>
+#include <linux/skbuff.h>
 #include <uapi/linux/ppp_defs.h>
 
 #define PPP_FCS(fcs, c) crc_ccitt_byte(fcs, c)
 
+/**
+ * ppp_skb_is_compressed_proto - checks if PPP protocol in a skb is compressed
+ * @skb: skb to check
+ *
+ * Returns true iff the PPP protocol field is compressed (the least significant
+ * bit of the most significant octet is 1).
+ * skb->data must point to the PPP protocol header.
+ */
+static inline bool ppp_skb_is_compressed_proto(const struct sk_buff *skb)
+{
+	return unlikely(skb->data[0] & 0x01);
+}
+
 /**
  * ppp_proto_is_valid - checks if PPP protocol is valid
  * @proto: PPP protocol
-- 
2.43.0

Re: [PATCH net v2] pppoe: drop PFC frames

[Simon Horman]

On Wed, Apr 08, 2026 at 10:42:39AM +0800, Qingfang Deng wrote:
> RFC 2516 Section 7 states that Protocol Field Compression (PFC) is NOT
> RECOMMENDED for PPPoE. In practice, pppd does not support negotiating
> PFC for PPPoE sessions, and the current PPPoE driver assumes an
> uncompressed (2-byte) protocol field. However, the generic PPP layer
> function ppp_input() is not aware of the negotiation result, and still
> accepts PFC frames.
> 
> If a peer with a broken implementation or an attacker sends a frame with
> a compressed (1-byte) protocol field, the subsequent PPP payload is
> shifted by one byte. This causes the network header to be 4-byte
> misaligned, which may trigger unaligned access exceptions on some
> architectures.
> 
> To reduce the attack surface, drop PPPoE PFC frames. Introduce
> ppp_skb_is_compressed_proto() helper function to be used in both
> ppp_generic.c and pppoe.c to avoid open-coding.
> 
> Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers")

AI generated review points out that the commit cited above only moves code
around. And thus while it may show up in git annotate as the source
of the buggy lines of code, actually it predates that commit.

AI generated code suggests an earlier commit that adds
the length check. But I believe that the bug predates that too.
And that the bug is present, in some form, since the beginning of
git history, because at that time fields of the ppp header was read without
taking PFC into account.

So I suggest:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Unless there is other review you probably don't need to repost just
to address this.

> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
> v2: retarget to net, and add a helper function
>  https://lore.kernel.org/netdev/20260403083926.68320-1-qingfang.deng@linux.dev/

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v2] pppoe: drop PFC frames

[Simon Horman]

On Fri, Apr 10, 2026 at 05:44:42PM +0100, Simon Horman wrote:
> On Wed, Apr 08, 2026 at 10:42:39AM +0800, Qingfang Deng wrote:
> > RFC 2516 Section 7 states that Protocol Field Compression (PFC) is NOT
> > RECOMMENDED for PPPoE. In practice, pppd does not support negotiating
> > PFC for PPPoE sessions, and the current PPPoE driver assumes an
> > uncompressed (2-byte) protocol field. However, the generic PPP layer
> > function ppp_input() is not aware of the negotiation result, and still
> > accepts PFC frames.
> > 
> > If a peer with a broken implementation or an attacker sends a frame with
> > a compressed (1-byte) protocol field, the subsequent PPP payload is
> > shifted by one byte. This causes the network header to be 4-byte
> > misaligned, which may trigger unaligned access exceptions on some
> > architectures.
> > 
> > To reduce the attack surface, drop PPPoE PFC frames. Introduce
> > ppp_skb_is_compressed_proto() helper function to be used in both
> > ppp_generic.c and pppoe.c to avoid open-coding.
> > 
> > Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers")
> 
> AI generated review points out that the commit cited above only moves code
> around. And thus while it may show up in git annotate as the source
> of the buggy lines of code, actually it predates that commit.
> 
> AI generated code suggests an earlier commit that adds
> the length check. But I believe that the bug predates that too.
> And that the bug is present, in some form, since the beginning of
> git history, because at that time fields of the ppp header was read without
> taking PFC into account.
> 
> So I suggest:
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
> Unless there is other review you probably don't need to repost just
> to address this.

Sorry, I now see there is a v3 and v4.
I will follow-up there.

-- 
pw-bot: superseded