netfilter: nf_queue: hold bridge skb->dev while queued

br_pass_frame_up() rewrites skb->dev from the ingress port to the bridge
master before queueing bridge LOCAL_IN packets. NFQUEUE only holds
references on state.in/out and bridge physdevs, so a queued bridge
packet can retain a freed bridge master in skb->dev until reinjection.

When the verdict is reinjected later, br_netif_receive_skb() re-enters
the receive path with skb->dev still pointing at the freed bridge master,
triggering a use-after-free.

Store skb->dev in the queue entry, hold a reference on it for the queue
lifetime, and use the saved device when dropping queued packets during
NETDEV_DOWN handling.

Fixes: ac2863445686 ("netfilter: bridge: add nf_afinfo to enable queuing to userspace")
Cc: stable@kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Signed-off-by: Haoze Xie <royenheart@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
index d17035d14d96cff132f0fa96ec5858760029d57c..3978c3174cdbe0c2329fe4dde28342e619c96d8b 100644 (file)
--- a/include/net/netfilter/nf_queue.h
+++ b/include/net/netfilter/nf_queue.h
@@ -14,6 +14,7 @@ struct nf_queue_entry {
        struct list_head        list;
        struct rhash_head       hash_node;
        struct sk_buff          *skb;
+       struct net_device       *skb_dev;
        unsigned int            id;
        unsigned int            hook_index;     /* index in hook_entries->hook[] */
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)

diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index a6c81c04b3a52a03a7c4dfcdfd0543cab3cc0f3d..57b450024a99ea0b2a4ea7516c6c6e5ceac36e42 100644 (file)
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -61,6 +61,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry)
        struct nf_hook_state *state = &entry->state;
 
        /* Release those devices we held, or Alexey will kill me. */
+       dev_put(entry->skb_dev);
        dev_put(state->in);
        dev_put(state->out);
        if (state->sk)
@@ -102,6 +103,7 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry)
        if (state->sk && !refcount_inc_not_zero(&state->sk->sk_refcnt))
                return false;
 
+       dev_hold(entry->skb_dev);
        dev_hold(state->in);
        dev_hold(state->out);
 
@@ -202,11 +204,11 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
 
        *entry = (struct nf_queue_entry) {
                .skb    = skb,
+               .skb_dev = skb->dev,
                .state  = *state,
                .hook_index = index,
                .size   = sizeof(*entry) + route_key_size,
        };
-
        __nf_queue_entry_init_physdevs(entry);
 
        if (!nf_queue_entry_get_refs(entry)) {

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 58304fd1f70ffde4fb617a517552288ae9154a63..984a0eb9e1492456afe91f89e2f025b70f287bb0 100644 (file)
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1212,6 +1212,8 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
        if (physinif == ifindex || physoutif == ifindex)
                return 1;
 #endif
+       if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
+               return 1;
        if (entry->state.in)
                if (entry->state.in->ifindex == ifindex)
                        return 1;