On 3/19/25 12:04 AM, Pablo Neira Ayuso wrote:
> Hi,
>
> On Sat, Mar 15, 2025 at 09:00:32PM +0100, Eric Woudstra wrote:
>> This adds the capability to evaluate 802.1ad, QinQ, PPPoE and PPPoE-in-Q
>> packets in the bridge filter chain.
>>
>> Reviewed-by: Nikolay Aleksandrov <razor@xxxxxxxxxxxxx>
>> Signed-off-by: Eric Woudstra <ericwouds@xxxxxxxxx>
>> ---
>> net/netfilter/nft_chain_filter.c | 20 +++++++++++++++++++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
>> index 19a553550c76..7c7080c1a67d 100644
>> --- a/net/netfilter/nft_chain_filter.c
>> +++ b/net/netfilter/nft_chain_filter.c
>> @@ -232,11 +232,27 @@ nft_do_chain_bridge(void *priv,
>> struct sk_buff *skb,
>> const struct nf_hook_state *state)
>> {
>> + struct ethhdr *ethh = eth_hdr(skb);
>> struct nft_pktinfo pkt;
>> + int thoff;
>>
>> nft_set_pktinfo(&pkt, skb, state);
>>
>> - switch (eth_hdr(skb)->h_proto) {
>> + switch (ethh->h_proto) {
>> + case htons(ETH_P_PPP_SES):
>> + thoff = PPPOE_SES_HLEN;
>> + ethh += thoff;
>
> This pointer arithmetics does not look correct, ethh is struct ethhdr,
> neither void nor char.
>
>> + break;
>> + case htons(ETH_P_8021Q):
>> + thoff = VLAN_HLEN;
>> + ethh += thoff;
>
> Same here.
>
>> + break;
>> + default:
>> + thoff = 0;
>> + break;
>> + }
>> +
>> + switch (ethh->h_proto) {
>
> This switch will match on the wrong offset.
>
>> case htons(ETH_P_IP):
>> nft_set_pktinfo_ipv4_validate(&pkt);
>> break;
>> @@ -248,6 +264,8 @@ nft_do_chain_bridge(void *priv,
>> break;
>> }
>>
>> + pkt.thoff += thoff;
>
> And only transport offset is adjusted here.
>
>> return nft_do_chain(&pkt, priv);
>> }
>>
>> --
>> 2.47.1
>>
I will sort this out and send a new version after the merge window.
