lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 22 Apr 2019 18:10:06 +0800 From: Rundong Ge <rdong.ge@...il.com> To: Florian Westphal <fw@...len.de> Cc: Pablo Neira Ayuso <pablo@...filter.org>, kadlec@...ckhole.kfki.hu, Roopa Prabhu <roopa@...ulusnetworks.com>, davem@...emloft.net, netfilter-devel@...r.kernel.org, coreteam@...filter.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] netfilter: fix dangling pointer access of fake_rtable The hook in my testcase is at NF_BR_FORWARD, and priority is -2. And at this hook point both the entry->out and entry->in are not bridge device. But the dst was set to the bridge's fake_rtable. Rundong Ge <rdong.ge@...il.com> 于2019年4月22日周一 下午5:51写道: > > skb->dev is munged in setup_prerouting() to be bridge or vlan device on > top of bridge. > --Yes, but br_nf_pre_routing_finish will set the skb->dev back to the phyindev. > > Florian Westphal <fw@...len.de> 于2019年4月22日周一 下午5:35写道: > > > > Rundong Ge <rdong.ge@...il.com> wrote: > > > br_nf_pre_routing will call the NF_INET_PRE_ROUTING hooks, at this > > > time both entry->state.in and entry->state.out are not bridge device. > > > > > > NF_HOOK(NFPROTO_IPV4, NF_INET_PRE_ROUTING, state->net, state->sk, skb, > > > skb->dev, NULL, > > > br_nf_pre_routing_finish); > > > > skb->dev is munged in setup_prerouting() to be bridge or vlan device on > > top of bridge. > > > > That being said, I think we need this fix at least: > > > > diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c > > --- a/net/netfilter/nf_queue.c > > +++ b/net/netfilter/nf_queue.c > > @@ -197,8 +197,15 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state, > > .size = sizeof(*entry) + route_key_size, > > }; > > > > + if (skb_dst(skb)) { > > + skb_dst_force(skb); > > + if (!skb_dst(skb)) { > > + status = -EHOSTUNREACH; > > + goto err; > > + } > > + } > > + > > nf_queue_entry_get_refs(entry); > > - skb_dst_force(skb); > > > > switch (entry->state.pf) { > > case AF_INET: > > > > > > Then, why not add, in dev_cmp: > > > > dst = skb_dst(skb); > > if (dst && dst->dev->index == index ... > > > > ?
Powered by blists - more mailing lists