diff mbox series

[Openvpn-devel,ovpn-net-next] ovpn: reset MAC header before passing skb up

Message ID 20260427040011.2107748-1-qingfang.deng@linux.dev
State New
Headers
Series [Openvpn-devel,ovpn-net-next] ovpn: reset MAC header before passing skb up |

Commit Message

Qingfang Deng April 27, 2026, 4 a.m. UTC 
  After decapsulating a packet, the skb->mac_header still points to the
outer transport header. Call skb_reset_mac_header() in
ovpn_netdev_write() to ensure the MAC header points to the beginning of
the inner IP packet.

Reported-by: Minqiang Chen <ptpt52@gmail.com>
Fixes: 8534731dbf2d ("ovpn: implement packet processing")
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ovpn/io.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Antonio Quartulli April 27, 2026, 9:45 a.m. UTC | #1 
Hi Qingfang,

thanks for the patch!

On 27/04/2026 06:00, Qingfang Deng wrote:
> After decapsulating a packet, the skb->mac_header still points to the
> outer transport header. Call skb_reset_mac_header() in
> ovpn_netdev_write() to ensure the MAC header points to the beginning of
> the inner IP packet.

May you elaborate on what this is exactly fixing?
Did you encounter a bug triggered by this missing line?

I am asking because I wonder what is "expected" as MAC header for a 
packet not having one at all (packets delivered to the ovpn interface 
are L3 only, as per the interface type itself).

Thanks!

Regards,

> 
> Reported-by: Minqiang Chen <ptpt52@gmail.com>
> Fixes: 8534731dbf2d ("ovpn: implement packet processing")
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
>   drivers/net/ovpn/io.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
> index db43a1f8a07a..d92bb87be2b2 100644
> --- a/drivers/net/ovpn/io.c
> +++ b/drivers/net/ovpn/io.c
> @@ -85,6 +85,7 @@ static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
>   	skb_scrub_packet(skb, true);
>   
>   	/* network header reset in ovpn_decrypt_post() */
> +	skb_reset_mac_header(skb);
>   	skb_reset_transport_header(skb);
>   	skb_reset_inner_headers(skb);
>
Qingfang Deng April 28, 2026, 2:08 a.m. UTC | #2 
Hi,

On 2026/4/27 17:45, Antonio Quartulli wrote:
> Hi Qingfang,
>
> thanks for the patch!
>
> On 27/04/2026 06:00, Qingfang Deng wrote:
>> After decapsulating a packet, the skb->mac_header still points to the
>> outer transport header. Call skb_reset_mac_header() in
>> ovpn_netdev_write() to ensure the MAC header points to the beginning of
>> the inner IP packet.
>
> May you elaborate on what this is exactly fixing?
> Did you encounter a bug triggered by this missing line?
>
> I am asking because I wonder what is "expected" as MAC header for a 
> packet not having one at all (packets delivered to the ovpn interface 
> are L3 only, as per the interface type itself).

For L3-only devices, the net core expects skb->mac_header == 
skb->network_header.

For example, in __netif_receive_skb_core(), skb_reset_mac_len() sets 
skb->mac_len to (skb->network_header - skb->mac_header).
If skb->mac_header still has a stale value, this will incorrectly assign 
a non-zero value to skb->mac_len.

Also, if generic XDP or SOCK_PACKET is used, either will do
   skb_push(skb, skb->data - skb_mac_header(skb));

>
> Thanks!
>
> Regards,
>
>>
>> Reported-by: Minqiang Chen <ptpt52@gmail.com>
>> Fixes: 8534731dbf2d ("ovpn: implement packet processing")
>> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
>> ---
>>   drivers/net/ovpn/io.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
>> index db43a1f8a07a..d92bb87be2b2 100644
>> --- a/drivers/net/ovpn/io.c
>> +++ b/drivers/net/ovpn/io.c
>> @@ -85,6 +85,7 @@ static void ovpn_netdev_write(struct ovpn_peer 
>> *peer, struct sk_buff *skb)
>>       skb_scrub_packet(skb, true);
>>         /* network header reset in ovpn_decrypt_post() */
>> +    skb_reset_mac_header(skb);
>>       skb_reset_transport_header(skb);
>>       skb_reset_inner_headers(skb);
>
+Cc: netdev

Regards,

Qingfang
Antonio Quartulli April 28, 2026, 9:36 a.m. UTC | #3 
Hi,

On 28/04/2026 04:08, Qingfang Deng wrote:
> Hi,
> 
> On 2026/4/27 17:45, Antonio Quartulli wrote:
>> Hi Qingfang,
>>
>> thanks for the patch!
>>
>> On 27/04/2026 06:00, Qingfang Deng wrote:
>>> After decapsulating a packet, the skb->mac_header still points to the
>>> outer transport header. Call skb_reset_mac_header() in
>>> ovpn_netdev_write() to ensure the MAC header points to the beginning of
>>> the inner IP packet.
>>
>> May you elaborate on what this is exactly fixing?
>> Did you encounter a bug triggered by this missing line?
>>
>> I am asking because I wonder what is "expected" as MAC header for a 
>> packet not having one at all (packets delivered to the ovpn interface 
>> are L3 only, as per the interface type itself).
> 
> For L3-only devices, the net core expects skb->mac_header == skb- 
>  >network_header.
> 
> For example, in __netif_receive_skb_core(), skb_reset_mac_len() sets 
> skb->mac_len to (skb->network_header - skb->mac_header).
> If skb->mac_header still has a stale value, this will incorrectly assign 
> a non-zero value to skb->mac_len.
> 
> Also, if generic XDP or SOCK_PACKET is used, either will do
>    skb_push(skb, skb->data - skb_mac_header(skb));

Thanks a lot! This makes sense!

I'm applying the patch to my tree.

Regards,
diff mbox series

Patch

diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index db43a1f8a07a..d92bb87be2b2 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -85,6 +85,7 @@  static void ovpn_netdev_write(struct ovpn_peer *peer, struct sk_buff *skb)
 	skb_scrub_packet(skb, true);
 
 	/* network header reset in ovpn_decrypt_post() */
+	skb_reset_mac_header(skb);
 	skb_reset_transport_header(skb);
 	skb_reset_inner_headers(skb);