netfilter: nf_dup_netdev: add nf_dev_xmit_recursion*() helpers and use them

Update nft_dup and nft_fwd to use the nf_dev_xmit_recursion() helpers.
This patch also disables BH when transmitting the skb to address a
possible migration to different CPU leading to imbalanced decrementation
of the recursion counters.

This is modeled after Florian Westphal's dev_xmit_recursion*() API
available since commit 97cdcf37b57e ("net: place xmit recursion in
softnet data") according to its current state in the tree.

Fixes: 1d47b55b36d2 ("netfilter: nft_fwd_netdev: use recursion counter in neigh egress path")
Fixes: f37ad9127039 ("netfilter: nf_dup_netdev: Move the recursion counter struct netdev_xmit")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/netfilter/nf_dup_netdev.h b/include/net/netfilter/nf_dup_netdev.h
index 609bcf422a9b31001b3adab31fb0717a00f31345..f6b05bd80c3fe38f09a17bb0be88dc5832e0ba30 100644 (file)
--- a/include/net/netfilter/nf_dup_netdev.h
+++ b/include/net/netfilter/nf_dup_netdev.h
@@ -11,15 +11,39 @@ void nf_fwd_netdev_egress(const struct nft_pktinfo *pkt, int oif);
 
 #define NF_RECURSION_LIMIT     2
 
-static inline u8 *nf_get_nf_dup_skb_recursion(void)
-{
 #ifndef CONFIG_PREEMPT_RT
-       return this_cpu_ptr(&softnet_data.xmit.nf_dup_skb_recursion);
+static inline bool nf_dev_xmit_recursion(void)
+{
+       return unlikely(__this_cpu_read(softnet_data.xmit.nf_dup_skb_recursion) >
+                       NF_RECURSION_LIMIT);
+}
+
+static inline void nf_dev_xmit_recursion_inc(void)
+{
+       __this_cpu_inc(softnet_data.xmit.nf_dup_skb_recursion);
+}
+
+static inline void nf_dev_xmit_recursion_dec(void)
+{
+       __this_cpu_dec(softnet_data.xmit.nf_dup_skb_recursion);
+}
 #else
-       return &current->net_xmit.nf_dup_skb_recursion;
-#endif
+static inline bool nf_dev_xmit_recursion(void)
+{
+       return unlikely(current->net_xmit.nf_dup_skb_recursion > NF_RECURSION_LIMIT);
+}
+
+static inline void nf_dev_xmit_recursion_inc(void)
+{
+       current->net_xmit.nf_dup_skb_recursion++;
 }
 
+static inline void nf_dev_xmit_recursion_dec(void)
+{
+       current->net_xmit.nf_dup_skb_recursion--;
+}
+#endif
+
 struct nft_offload_ctx;
 struct nft_flow_rule;
 

diff --git a/net/netfilter/nf_dup_netdev.c b/net/netfilter/nf_dup_netdev.c
index 3b0a70e154cd8fcc6d96d531902e60fdfb2b69bc..c189716e986a328a1d9a7a3680f50aec3a3a2d16 100644 (file)
--- a/net/netfilter/nf_dup_netdev.c
+++ b/net/netfilter/nf_dup_netdev.c
@@ -16,11 +16,6 @@
 static void nf_do_netdev_egress(struct sk_buff *skb, struct net_device *dev,
                                enum nf_dev_hooks hook)
 {
-       u8 *nf_dup_skb_recursion = nf_get_nf_dup_skb_recursion();
-
-       if (*nf_dup_skb_recursion > NF_RECURSION_LIMIT)
-               goto err;
-
        if (hook == NF_NETDEV_INGRESS && skb_mac_header_was_set(skb)) {
                if (skb_cow_head(skb, skb->mac_len))
                        goto err;
@@ -30,9 +25,15 @@ static void nf_do_netdev_egress(struct sk_buff *skb, struct net_device *dev,
 
        skb->dev = dev;
        skb_clear_tstamp(skb);
-       (*nf_dup_skb_recursion)++;
+       local_bh_disable();
+       if (nf_dev_xmit_recursion()) {
+               local_bh_enable();
+               goto err;
+       }
+       nf_dev_xmit_recursion_inc();
        dev_queue_xmit(skb);
-       (*nf_dup_skb_recursion)--;
+       nf_dev_xmit_recursion_dec();
+       local_bh_enable();
        return;
 err:
        kfree_skb(skb);

diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
index b9e88d7cf3081adcded40450ee4fd820551dcfa0..a48c2f765bbaadb85266ee9aa25d20a372f93daf 100644 (file)
--- a/net/netfilter/nft_fwd_netdev.c
+++ b/net/netfilter/nft_fwd_netdev.c
@@ -95,7 +95,6 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
                              struct nft_regs *regs,
                              const struct nft_pktinfo *pkt)
 {
-       u8 *nf_dup_skb_recursion = nf_get_nf_dup_skb_recursion();
        struct nft_fwd_neigh *priv = nft_expr_priv(expr);
        void *addr = &regs->data[priv->sreg_addr];
        int oif = regs->data[priv->sreg_dev];
@@ -154,13 +153,15 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
                goto out;
        }
 
-       if (*nf_dup_skb_recursion > NF_RECURSION_LIMIT) {
+       dev = dev_get_by_index_rcu(nft_net(pkt), oif);
+       if (!dev) {
                verdict = NF_DROP;
                goto out;
        }
 
-       dev = dev_get_by_index_rcu(nft_net(pkt), oif);
-       if (dev == NULL) {
+       local_bh_disable();
+       if (nf_dev_xmit_recursion()) {
+               local_bh_enable();
                verdict = NF_DROP;
                goto out;
        }
@@ -169,16 +170,18 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
        if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) {
                skb = skb_expand_head(skb, hh_len);
                if (!skb) {
-                       verdict = NF_STOLEN;
+                       local_bh_enable();
                        goto out;
                }
        }
 
        skb->dev = dev;
        skb_clear_tstamp(skb);
-       (*nf_dup_skb_recursion)++;
+
+       nf_dev_xmit_recursion_inc();
        neigh_xmit(neigh_table, dev, addr, skb);
-       (*nf_dup_skb_recursion)--;
+       nf_dev_xmit_recursion_dec();
+       local_bh_enable();
 out:
        regs->verdict.code = verdict;
 }