netfilter: nf_queue: fix possible use-after-free

commit c3873070247d9e3c7a6b0cf9bf9b45e8018427b1 upstream.

Eric Dumazet says:
  The sock_hold() side seems suspect, because there is no guarantee
  that sk_refcnt is not already 0.

On failure, we cannot queue the packet and need to indicate an
error.  The packet will be dropped by the caller.

v2: split skb prefetch hunk into separate change

Fixes: 271b72c7fa82c ("udp: RCU handling for Unicast packets.")
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
index e770bba00066453885c147cfebaf2f3ed6f00124..b1d43894296a6ac35b9e69e90efa13f32ba9143e 100644 (file)
--- a/include/net/netfilter/nf_queue.h
+++ b/include/net/netfilter/nf_queue.h
@@ -37,7 +37,7 @@ void nf_register_queue_handler(struct net *net, const struct nf_queue_handler *q
 void nf_unregister_queue_handler(struct net *net);
 void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict);
 
-void nf_queue_entry_get_refs(struct nf_queue_entry *entry);
+bool nf_queue_entry_get_refs(struct nf_queue_entry *entry);
 void nf_queue_entry_free(struct nf_queue_entry *entry);
 
 static inline void init_hashrandom(u32 *jhash_initval)

diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 897bbd686cd96ffe1bf20db175fd8cf37599a5c6..543904249f7e11a6a80a4fa1f37bc09483560f6f 100644 (file)
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -100,16 +100,17 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
 }
 
 /* Bump dev refs so they don't vanish while packet is out */
-void nf_queue_entry_get_refs(struct nf_queue_entry *entry)
+bool nf_queue_entry_get_refs(struct nf_queue_entry *entry)
 {
        struct nf_hook_state *state = &entry->state;
 
+       if (state->sk && !refcount_inc_not_zero(&state->sk->sk_refcnt))
+               return false;
+
        if (state->in)
                dev_hold(state->in);
        if (state->out)
                dev_hold(state->out);
-       if (state->sk)
-               sock_hold(state->sk);
 
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
        if (entry->physin)
@@ -117,6 +118,7 @@ void nf_queue_entry_get_refs(struct nf_queue_entry *entry)
        if (entry->physout)
                dev_hold(entry->physout);
 #endif
+       return true;
 }
 EXPORT_SYMBOL_GPL(nf_queue_entry_get_refs);
 
@@ -205,7 +207,10 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
 
        __nf_queue_entry_init_physdevs(entry);
 
-       nf_queue_entry_get_refs(entry);
+       if (!nf_queue_entry_get_refs(entry)) {
+               kfree(entry);
+               return -ENOTCONN;
+       }
 
        switch (entry->state.pf) {
        case AF_INET:

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index b0358f30947ea80b6aa715bd515d652538ee785e..1640da5c50776189b8cfc354eb5e6d09ccc50363 100644 (file)
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -712,9 +712,15 @@ static struct nf_queue_entry *
 nf_queue_entry_dup(struct nf_queue_entry *e)
 {
        struct nf_queue_entry *entry = kmemdup(e, e->size, GFP_ATOMIC);
-       if (entry)
-               nf_queue_entry_get_refs(entry);
-       return entry;
+
+       if (!entry)
+               return NULL;
+
+       if (nf_queue_entry_get_refs(entry))
+               return entry;
+
+       kfree(entry);
+       return NULL;
 }
 
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)