netdev - Re: [PATCH] netfilter: fix dangling pointer access of fake

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190422093502.36da6z3qf7zxpwny@breakpoint.cc>
Date:   Mon, 22 Apr 2019 11:35:02 +0200
From:   Florian Westphal <fw@...len.de>
To:     Rundong Ge <rdong.ge@...il.com>
Cc:     Pablo Neira Ayuso <pablo@...filter.org>, kadlec@...ckhole.kfki.hu,
        fw@...len.de, 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

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 ...

?