Date:   Sun, 24 Apr 2022 22:57:10 +0200
From:   <alexandre.ferrieux@...nge.com>
To:     <netdev@...r.kernel.org>
Subject: Re: Zero-Day bug in VLAN offloading + cooked AF_PACKET

On 4/23/22 22:02, alexandre.ferrieux@...nge.com wrote:
> 
> TL;DR: outgoing VLAN-tagged traffic to non-offloaded interfaces is captured as 
> corrupted in cooked mode, and has been so since at least 3.4...
> 
> [...]
> 
> However, there's a catch: for outgoing packets, *if* the interface has no 
> hardware VLAN offloading, the ethertype gets overwritten by ... the TPID 
> (0x8100). As a result, a consumer of the L3 frame has absolutely no way to 
> recover its type.

Digging a bit shows that the key is a discrepancy between where "layer 2.5" sits 
wrt the link/network boundary.

Indeed:

  - to L3 abstractions, the VLAN belongs to "link layer" and should be mostly 
ignored: skb->protocol and skb->network_header rule the world.

  - to (software) VLAN insertion code "__vlan_hwaccel_push_inside", when a frame 
is prepared for transmission by a non-vlan-offload-capable device, the link 
layer stops right after the MAC adresses, the TPID is provided as an ethertype 
and written into skb->protocol, and the original ethertype is explicitly stacked 
after the TCI. In other words, 802.1Q is a kind of layer 3 (though not 
completely: skb->network_header is *not* set to the TPID position - it remains 
at the true L3).

This discrepancy is mostly harmless, except in the presence of a tap: there 
(e.g. in packet_rcv), on the tx path in cooked (SOCK_DGRAM) mode, the packet is 
stripped of everything before its skb->network_header:

   if (sk->sk_type != SOCK_DGRAM)
     skb_push(skb, skb->data - skb_mac_header(skb));
   else if (skb->pkt_type == PACKET_OUTGOING) {
     /* Special case: outgoing packets have ll header at head */
     skb_pull(skb, skb_network_offset(skb));
   }

As a result, the original ethertype is lost, with the effect we know.

Now I can see two ways out:

  (A) "Fix" _vlan_hwaccel_push_inside to update skb->network_header 
(decrementing it by 4 bytes), to preserve the invariant "skb->protocol describes 
what starts at skb->network_header".

  (B) Refine the stripping in packet_rcv (and brothers) to special-case VLANs, 
effectively re-parsing what we've just inserted.

To me, (A) is cleanest as it enforces a broken invariant; however, it possibly 
modifies the tx path in ways I cannot fathom. Conversely, (B) looks like an ugly 
hack whose sole value is to be confined to tap-cloned skbs, bounding the harm it 
may cause, but with a bit of CPU waste...

Please advise :)

-Alex




_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

Powered by blists - more mailing lists