netdev - Re: [Patch net-next] ipv4: initialize ra_mutex in inet_init

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: <be6bbcbd-0817-691a-78b8-74b9b5b0a8b0@virtuozzo.com>
Date:   Mon, 17 Sep 2018 10:25:05 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org
Subject: Re: [Patch net-next] ipv4: initialize ra_mutex in inet_init_net()

On 14.09.2018 23:32, Cong Wang wrote:
> ra_mutex is a IPv4 specific mutex, it is inside struct netns_ipv4,
> but its initialization is in the generic netns code, setup_net().
> 
> Move it to IPv4 specific net init code, inet_init_net().
> 
> Fixes: d9ff3049739e ("net: Replace ip_ra_lock with per-net mutex")
> Cc: Kirill Tkhai <ktkhai@...tuozzo.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> ---
>  net/core/net_namespace.c | 1 -
>  net/ipv4/af_inet.c       | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 670c84b1bfc2..b272ccfcbf63 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -308,7 +308,6 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
>  	net->user_ns = user_ns;
>  	idr_init(&net->netns_ids);
>  	spin_lock_init(&net->nsid_lock);
> -	mutex_init(&net->ipv4.ra_mutex);
>  
>  	list_for_each_entry(ops, &pernet_list, list) {
>  		error = ops_init(ops, net);
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 20fda8fb8ffd..57b7bffb93e5 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1817,6 +1817,8 @@ static __net_init int inet_init_net(struct net *net)
>  	net->ipv4.sysctl_igmp_llm_reports = 1;
>  	net->ipv4.sysctl_igmp_qrv = 2;
>  
> +	mutex_init(&net->ipv4.ra_mutex);
> +

In inet_init() the order of registration is:

	ip_mr_init();
	init_inet_pernet_ops();

This means, ipmr_net_ops pernet operations are before af_inet_ops
in pernet_list. So, there is a theoretical probability, sometimes
in the future, we will have a problem during a fail of net initialization.

Say,

setup_net():
	ipmr_net_ops->init() returns 0
	xxx->init()          returns error
and then we do:
	ipmr_net_ops->exit(),

which could touch ra_mutex (theoretically).

Your patch is OK, but since you do this, we may also swap the order
of registration of ipmr_net_ops and af_inet_ops better too.

Kirill