| Message ID | dcf5953ca3d90bddf3b496d7bb86c9d30f7069f4.1500381553.git.sd@queasysnail.net |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Jul 19, 2017 at 12:56 AM, Sabrina Dubroca <sd@queasysnail.net> wrote: > When we delete a netns with a CLUSTERIP rule, clusterip_net_exit() is > called first, removing /proc/net/ipt_CLUSTERIP. > Then clusterip_config_entry_put() is called from clusterip_tg_destroy(), > and tries to remove its entry under /proc/net/ipt_CLUSTERIP/. > > Fix this by checking that the parent directory of the entry to remove > hasn't already been deleted. > > The following triggers a KASAN splat (stealing the reproducer from > 202f59afd441, thanks to Jianlin Shi and Xin Long): > > ip netns add test > ip link add veth0_in type veth peer name veth0_out > ip link set veth0_in netns test > ip netns exec test ip link set lo up > ip netns exec test ip link set veth0_in up > ip netns exec test iptables -I INPUT -d 1.2.3.4 -i veth0_in -j \ > CLUSTERIP --new --clustermac 89:d4:47:eb:9a:fa --total-nodes 3 \ > --local-node 1 --hashmode sourceip-sourceport > ip netns del test > > Fixes: ce4ff76c15a8 ("netfilter: ipt_CLUSTERIP: make proc directory per net namespace") > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Reviewed-by: Xin Long <lucien.xin@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 19, 2017 at 12:29:58PM +1200, Xin Long wrote: > On Wed, Jul 19, 2017 at 12:56 AM, Sabrina Dubroca <sd@queasysnail.net> wrote: > > When we delete a netns with a CLUSTERIP rule, clusterip_net_exit() is > > called first, removing /proc/net/ipt_CLUSTERIP. > > Then clusterip_config_entry_put() is called from clusterip_tg_destroy(), > > and tries to remove its entry under /proc/net/ipt_CLUSTERIP/. > > > > Fix this by checking that the parent directory of the entry to remove > > hasn't already been deleted. > > > > The following triggers a KASAN splat (stealing the reproducer from > > 202f59afd441, thanks to Jianlin Shi and Xin Long): > > > > ip netns add test > > ip link add veth0_in type veth peer name veth0_out > > ip link set veth0_in netns test > > ip netns exec test ip link set lo up > > ip netns exec test ip link set veth0_in up > > ip netns exec test iptables -I INPUT -d 1.2.3.4 -i veth0_in -j \ > > CLUSTERIP --new --clustermac 89:d4:47:eb:9a:fa --total-nodes 3 \ > > --local-node 1 --hashmode sourceip-sourceport > > ip netns del test > > > > Fixes: ce4ff76c15a8 ("netfilter: ipt_CLUSTERIP: make proc directory per net namespace") > > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> > > Reviewed-by: Xin Long <lucien.xin@gmail.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c index 7d72decb80f9..efaa04dcc80e 100644 --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c @@ -117,7 +117,8 @@ clusterip_config_entry_put(struct net *net, struct clusterip_config *c) * functions are also incrementing the refcount on their own, * so it's safe to remove the entry even if it's in use. */ #ifdef CONFIG_PROC_FS - proc_remove(c->pde); + if (cn->procdir) + proc_remove(c->pde); #endif return; } @@ -815,6 +816,7 @@ static void clusterip_net_exit(struct net *net) #ifdef CONFIG_PROC_FS struct clusterip_net *cn = net_generic(net, clusterip_net_id); proc_remove(cn->procdir); + cn->procdir = NULL; #endif nf_unregister_net_hook(net, &cip_arp_ops); }
When we delete a netns with a CLUSTERIP rule, clusterip_net_exit() is called first, removing /proc/net/ipt_CLUSTERIP. Then clusterip_config_entry_put() is called from clusterip_tg_destroy(), and tries to remove its entry under /proc/net/ipt_CLUSTERIP/. Fix this by checking that the parent directory of the entry to remove hasn't already been deleted. The following triggers a KASAN splat (stealing the reproducer from 202f59afd441, thanks to Jianlin Shi and Xin Long): ip netns add test ip link add veth0_in type veth peer name veth0_out ip link set veth0_in netns test ip netns exec test ip link set lo up ip netns exec test ip link set veth0_in up ip netns exec test iptables -I INPUT -d 1.2.3.4 -i veth0_in -j \ CLUSTERIP --new --clustermac 89:d4:47:eb:9a:fa --total-nodes 3 \ --local-node 1 --hashmode sourceip-sourceport ip netns del test Fixes: ce4ff76c15a8 ("netfilter: ipt_CLUSTERIP: make proc directory per net namespace") Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> --- net/ipv4/netfilter/ipt_CLUSTERIP.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)