On Thu, Jan 25, 2018 at 2:00 AM, Pablo Neira Ayuso <pa...@netfilter.org> wrote: > On Wed, Jan 24, 2018 at 04:37:16PM -0500, David Miller wrote: >> From: Eyal Birger <eyal.bir...@gmail.com> >> Date: Tue, 23 Jan 2018 11:17:32 +0200 >> >> > + network_offset = skb_network_offset(skb); >> > + skb_pull(skb, network_offset); >> > + >> > + rcu_read_lock(); >> > + >> > + if (skb->skb_iif) >> > + indev = dev_get_by_index_rcu(em->net, skb->skb_iif); >> > + >> > + nf_hook_state_init(&state, im->hook, im->nfproto, indev ?: skb->dev, >> > + skb->dev, NULL, em->net, NULL); >> > + >> > + acpar.match = im->match; >> > + acpar.matchinfo = im->match_data; >> > + acpar.state = &state; >> > + >> > + ret = im->match->match(skb, &acpar); >> > + >> > + rcu_read_unlock(); >> > + >> > + skb_push(skb, network_offset); >> >> If the SKB is shared in any way, this pull/push around the NF hook >> invocation is illegal. > > At ingress, skb->data points to the network header, which is what the > xtables matches expect, so these are actually noops, therefore, > skb_pull() and skb_push() can be removed.
Right. I added those for completeness in supporting the xmit path. In the xt_policy use-case it is irrelevant as tc is invoked after encapsulation. I will submit a v2 without these, asserting the ingress direction. Note I have followed the example in em_ipset.c, so that might be a problem there too... Thanks! Eyal.
