netdev - Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171113072159.GN5512@breakpoint.cc>
Date:   Mon, 13 Nov 2017 08:21:59 +0100
From:   Florian Westphal <fw@...len.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Florian Westphal <fw@...len.de>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/8] rtnetlink: add rtnl_register_module

Peter Zijlstra <peterz@...radead.org> wrote:
> On Tue, Nov 07, 2017 at 10:47:51AM +0100, Florian Westphal wrote:
> > I would expect this to trigger all the time, due to
> > 
> > rtnl_register(AF_INET, RTM_GETROUTE, ...
> > rtnl_register(AF_INET, RTM_GETADDR, ...
> 
> Ah, sure, then something like so then...
> 
> There's bound to be bugs there too, as I pretty much typed this without
> thinking, but it should show the idea.

Just o let you know, I am backlogged at the moment so I Will not have
time to work on this for the time being.

> ---
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5ace48926b19..de1336775602 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -63,6 +63,7 @@ struct rtnl_link {
>  	rtnl_doit_func		doit;
>  	rtnl_dumpit_func	dumpit;
>  	unsigned int		flags;
> +	struct rcu_head		rcu;
>  };

This will need to be split:

struct rtnl_link {
	rtnl_doit_func          doit;
	unsigned int		flags;
	struct rcu_head		rcu;
};
struct rtnl_link_dump {
  	rtnl_dumpit_func	dumpit;
	struct rcu_head		rcu;
};

> -static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
> +static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];

So this will need to be two arrays.

Reason is that some places do this:

rtnl_register(pf, RTM_FOO, doit, NULL, 0);
rtnl_register(pf, RTM_FOO, NULL, dumpit, 0);

(from different call sites in the stack).
> -	if (doit)
> -		tab[msgindex].doit = doit;
> -	if (dumpit)
> -		tab[msgindex].dumpit = dumpit;

Which is the reason for these if () tests.