| Message ID | 20240522231355.9802-5-pablo@netfilter.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [net,1/6] netfilter: nfnetlink_queue: acquire rcu_read_lock() in instance_destroy_rcu() | expand |
On Thu, 2024-05-23 at 01:13 +0200, Pablo Neira Ayuso wrote: > @@ -801,21 +801,79 @@ struct nft_payload_set { > u8 csum_flags; > }; > > +/* This is not struct vlan_hdr. */ > +struct nft_payload_vlan_hdr { > + __be16 h_vlan_proto; > + __be16 h_vlan_TCI; > +}; > + > +static bool > +nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len, > + int *vlan_hlen) > +{ > + struct nft_payload_vlan_hdr *vlanh; > + __be16 vlan_proto; > + __be16 vlan_tci; > + > + if (offset >= offsetof(struct vlan_ethhdr, h_vlan_encapsulated_proto)) { > + *vlan_hlen = VLAN_HLEN; > + return true; > + } > + > + switch (offset) { > + case offsetof(struct vlan_ethhdr, h_vlan_proto): > + if (len == 2) { > + vlan_proto = nft_reg_load16(src); I'm sorry but the above introduces build warning due to endianess mismatch (host -> be) > + skb->vlan_proto = vlan_proto; > + } else if (len == 4) { > + vlanh = (struct nft_payload_vlan_hdr *)src; > + __vlan_hwaccel_put_tag(skb, vlanh->h_vlan_proto, > + ntohs(vlanh->h_vlan_TCI)); > + } else { > + return false; > + } > + break; > + case offsetof(struct vlan_ethhdr, h_vlan_TCI): > + if (len != 2) > + return false; > + > + vlan_tci = ntohs(nft_reg_load16(src)); Similar things here htons() expect a be short int and is receiving a u16, vlan_tci is 'be' and the assigned data uses host endianess. Could you please address the above? Thanks! Paolo
On Thu, May 23, 2024 at 11:26:45AM +0200, Paolo Abeni wrote: > On Thu, 2024-05-23 at 01:13 +0200, Pablo Neira Ayuso wrote: > > @@ -801,21 +801,79 @@ struct nft_payload_set { > > u8 csum_flags; > > }; > > > > +/* This is not struct vlan_hdr. */ > > +struct nft_payload_vlan_hdr { > > + __be16 h_vlan_proto; > > + __be16 h_vlan_TCI; > > +}; > > + > > +static bool > > +nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len, > > + int *vlan_hlen) > > +{ > > + struct nft_payload_vlan_hdr *vlanh; > > + __be16 vlan_proto; > > + __be16 vlan_tci; > > + > > + if (offset >= offsetof(struct vlan_ethhdr, h_vlan_encapsulated_proto)) { > > + *vlan_hlen = VLAN_HLEN; > > + return true; > > + } > > + > > + switch (offset) { > > + case offsetof(struct vlan_ethhdr, h_vlan_proto): > > + if (len == 2) { > > + vlan_proto = nft_reg_load16(src); > > I'm sorry but the above introduces build warning due to endianess > mismatch (host -> be) > > > + skb->vlan_proto = vlan_proto; > > + } else if (len == 4) { > > + vlanh = (struct nft_payload_vlan_hdr *)src; > > + __vlan_hwaccel_put_tag(skb, vlanh->h_vlan_proto, > > + ntohs(vlanh->h_vlan_TCI)); > > + } else { > > + return false; > > + } > > + break; > > + case offsetof(struct vlan_ethhdr, h_vlan_TCI): > > + if (len != 2) > > + return false; > > + > > + vlan_tci = ntohs(nft_reg_load16(src)); > > Similar things here htons() expect a be short int and is receiving a > u16, vlan_tci is 'be' and the assigned data uses host endianess. > > > Could you please address the above? Sure, I will post v2.
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c index a3cb5dbcb362..e1af7b5e70c6 100644 --- a/net/netfilter/nft_payload.c +++ b/net/netfilter/nft_payload.c @@ -145,12 +145,12 @@ int nft_payload_inner_offset(const struct nft_pktinfo *pkt) return pkt->inneroff; } -static bool nft_payload_need_vlan_copy(const struct nft_payload *priv) +static bool nft_payload_need_vlan_adjust(u32 offset, u32 len) { - unsigned int len = priv->offset + priv->len; + unsigned int boundary = offset + len; /* data past ether src/dst requested, copy needed */ - if (len > offsetof(struct ethhdr, h_proto)) + if (boundary > offsetof(struct ethhdr, h_proto)) return true; return false; @@ -174,7 +174,7 @@ void nft_payload_eval(const struct nft_expr *expr, goto err; if (skb_vlan_tag_present(skb) && - nft_payload_need_vlan_copy(priv)) { + nft_payload_need_vlan_adjust(priv->offset, priv->len)) { if (!nft_payload_copy_vlan(dest, skb, priv->offset, priv->len)) goto err; @@ -801,21 +801,79 @@ struct nft_payload_set { u8 csum_flags; }; +/* This is not struct vlan_hdr. */ +struct nft_payload_vlan_hdr { + __be16 h_vlan_proto; + __be16 h_vlan_TCI; +}; + +static bool +nft_payload_set_vlan(const u32 *src, struct sk_buff *skb, u8 offset, u8 len, + int *vlan_hlen) +{ + struct nft_payload_vlan_hdr *vlanh; + __be16 vlan_proto; + __be16 vlan_tci; + + if (offset >= offsetof(struct vlan_ethhdr, h_vlan_encapsulated_proto)) { + *vlan_hlen = VLAN_HLEN; + return true; + } + + switch (offset) { + case offsetof(struct vlan_ethhdr, h_vlan_proto): + if (len == 2) { + vlan_proto = nft_reg_load16(src); + skb->vlan_proto = vlan_proto; + } else if (len == 4) { + vlanh = (struct nft_payload_vlan_hdr *)src; + __vlan_hwaccel_put_tag(skb, vlanh->h_vlan_proto, + ntohs(vlanh->h_vlan_TCI)); + } else { + return false; + } + break; + case offsetof(struct vlan_ethhdr, h_vlan_TCI): + if (len != 2) + return false; + + vlan_tci = ntohs(nft_reg_load16(src)); + skb->vlan_tci = vlan_tci; + break; + default: + return false; + } + + return true; +} + static void nft_payload_set_eval(const struct nft_expr *expr, struct nft_regs *regs, const struct nft_pktinfo *pkt) { const struct nft_payload_set *priv = nft_expr_priv(expr); - struct sk_buff *skb = pkt->skb; const u32 *src = ®s->data[priv->sreg]; - int offset, csum_offset; + int offset, csum_offset, vlan_hlen = 0; + struct sk_buff *skb = pkt->skb; __wsum fsum, tsum; switch (priv->base) { case NFT_PAYLOAD_LL_HEADER: if (!skb_mac_header_was_set(skb)) goto err; - offset = skb_mac_header(skb) - skb->data; + + if (skb_vlan_tag_present(skb) && + nft_payload_need_vlan_adjust(priv->offset, priv->len)) { + if (!nft_payload_set_vlan(src, skb, + priv->offset, priv->len, + &vlan_hlen)) + goto err; + + if (!vlan_hlen) + return; + } + + offset = skb_mac_header(skb) - skb->data - vlan_hlen; break; case NFT_PAYLOAD_NETWORK_HEADER: offset = skb_network_offset(skb);
Userspace assumes vlan header is present at a given offset, but vlan offload allows to store this in metadata fields of the skbuff. Hence mangling vlan results in a garbled packet. Handle this transparently by adding a parser to the kernel. If vlan metadata is present and payload offset is over 12 bytes (source and destination mac address fields), then subtract vlan header present in vlan metadata, otherwise mangle vlan metadata based on offset and length, extracting data from the source register. This is similar to: 8cfd23e67401 ("netfilter: nft_payload: work around vlan header stripping") to deal with vlan payload mangling. Fixes: 7ec3f7b47b8d ("netfilter: nft_payload: add packet mangling support") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nft_payload.c | 72 +++++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 7 deletions(-)