| Message ID | 20260512224417.812214-1-pablo@netfilter.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [nf] netfilter: nf_queue: hold reference on skb->dev | expand |
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > Before NF_BR_LOCAL_IN, skb->dev is mangled in a way that results in > state->in != skb->dev, which can result in UaF when accessing the bridge > device if removed while in the queue. > > Reported-by: Ren Wei <n05ec@lzu.edu.cn> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > net/netfilter/nf_queue.c | 6 ++++++ > net/netfilter/nfnetlink_queue.c | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c > index a6c81c04b3a5..9c6741673842 100644 > --- a/net/netfilter/nf_queue.c > +++ b/net/netfilter/nf_queue.c > @@ -59,8 +59,11 @@ static void nf_queue_sock_put(struct sock *sk) > static void nf_queue_entry_release_refs(struct nf_queue_entry *entry) > { > struct nf_hook_state *state = &entry->state; > + struct sk_buff *skb = entry->skb; > > /* Release those devices we held, or Alexey will kill me. */ > + if (skb->dev) > + dev_put(skb->dev); dev_hold/put(NULL) is safe, no need for NULL guard (and no need to resend). Thanks for making a patch. I'll make a patch for the pptp/gre bug in case reporter remains silent.
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index a6c81c04b3a5..9c6741673842 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -59,8 +59,11 @@ static void nf_queue_sock_put(struct sock *sk) static void nf_queue_entry_release_refs(struct nf_queue_entry *entry) { struct nf_hook_state *state = &entry->state; + struct sk_buff *skb = entry->skb; /* Release those devices we held, or Alexey will kill me. */ + if (skb->dev) + dev_put(skb->dev); dev_put(state->in); dev_put(state->out); if (state->sk) @@ -98,10 +101,13 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry) bool nf_queue_entry_get_refs(struct nf_queue_entry *entry) { struct nf_hook_state *state = &entry->state; + struct sk_buff *skb = entry->skb; if (state->sk && !refcount_inc_not_zero(&state->sk->sk_refcnt)) return false; + if (skb->dev) + dev_hold(skb->dev); dev_hold(state->in); dev_hold(state->out); diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 58304fd1f70f..7408b348da13 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -1212,6 +1212,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex) if (physinif == ifindex || physoutif == ifindex) return 1; #endif + if (entry->skb->dev) + if (entry->skb->dev->ifindex == ifindex) + return 1; if (entry->state.in) if (entry->state.in->ifindex == ifindex) return 1;
Before NF_BR_LOCAL_IN, skb->dev is mangled in a way that results in state->in != skb->dev, which can result in UaF when accessing the bridge device if removed while in the queue. Reported-by: Ren Wei <n05ec@lzu.edu.cn> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_queue.c | 6 ++++++ net/netfilter/nfnetlink_queue.c | 3 +++ 2 files changed, 9 insertions(+)