[PATCH net-next] pppoe: drop PFC frames

[PATCH net-next] 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.

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 the compressed protocol field frames.

Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ppp/pppoe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 1ac61c273b28..457a83c73293 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,10 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb->len < len)
 		goto drop;
 
+	/* drop PFC frames */
+	if (unlikely(skb->data[0] & 0x01))
+		goto drop;
+
 	if (pskb_trim_rcsum(skb, len))
 		goto drop;
 
-- 
2.43.0

Re: [PATCH net-next] pppoe: drop PFC frames

[Simon Horman]

On Fri, Apr 03, 2026 at 04:39:26PM +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.
> 
> 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 the compressed protocol field frames.
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
>  drivers/net/ppp/pppoe.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 1ac61c273b28..457a83c73293 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,10 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if (skb->len < len)
>  		goto drop;
>  
> +	/* drop PFC frames */
> +	if (unlikely(skb->data[0] & 0x01))
> +		goto drop;

Hi,

I think it would be best to add/use a #define rather than
open coding the magic value 0x01. And perhaps expanding
the comment to note that skb->data[0] is the first byte
of the PPP protocol would be nice too.

> +
>  	if (pskb_trim_rcsum(skb, len))
>  		goto drop;
>  
> -- 
> 2.43.0
> 

Re: [PATCH net-next] pppoe: drop PFC frames

[qingfang.deng]

Hi,


April 6, 2026 at 10:48 PM, Simon Horman wrote:
> 
> Hi,
> 
> I think it would be best to add/use a #define rather than
> open coding the magic value 0x01. And perhaps expanding
> the comment to note that skb->data[0] is the first byte
> of the PPP protocol would be nice too.

The field does not have a canonical name. As per RFC1661, the LSB of the 
first octet is used to test if the protocol field is compressed, and the 
same code snippet is used in ppp_generic.c.

I could instead add a helper function:

static inline bool ppp_skb_is_compressed_proto(const struct sk_buff *skb)
{
	return skb->data[0] & 0x01;
}

What do you think?

Regards,
Qingfang