netdev - Re: [patch net-next 03/15] net: fib_notifier: propagate possible error during fib notifier registration

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: <20190915081746.GB11194@splinter>
Date:   Sun, 15 Sep 2019 11:17:46 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, idosch@...lanox.com,
        dsahern@...il.com, jakub.kicinski@...ronome.com,
        tariqt@...lanox.com, saeedm@...lanox.com, kuznet@....inr.ac.ru,
        yoshfuji@...ux-ipv6.org, shuah@...nel.org, mlxsw@...lanox.com
Subject: Re: [patch net-next 03/15] net: fib_notifier: propagate possible
 error during fib notifier registration

On Sat, Sep 14, 2019 at 08:45:56AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...lanox.com>
> 
> Unlike events for registered notifier, during the registration, the
> errors that happened for the block being registered are not propagated
> up to the caller. For fib rules, this is already present, but not for

What do you mean by "already present" ? You added it below for rules as
well...

> fib entries. So make sure the error is propagated for those as well.
> 
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
>  include/net/ip_fib.h    |  2 +-
>  net/core/fib_notifier.c |  2 --
>  net/core/fib_rules.c    | 11 ++++++++---
>  net/ipv4/fib_notifier.c |  4 +---
>  net/ipv4/fib_trie.c     | 31 ++++++++++++++++++++++---------
>  net/ipv4/ipmr_base.c    | 22 +++++++++++++++-------
>  net/ipv6/ip6_fib.c      | 36 ++++++++++++++++++++++++------------
>  7 files changed, 71 insertions(+), 37 deletions(-)
> 
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 4cec9ecaa95e..caae0fa610aa 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -229,7 +229,7 @@ int __net_init fib4_notifier_init(struct net *net);
>  void __net_exit fib4_notifier_exit(struct net *net);
>  
>  void fib_info_notify_update(struct net *net, struct nl_info *info);
> -void fib_notify(struct net *net, struct notifier_block *nb);
> +int fib_notify(struct net *net, struct notifier_block *nb);
>  
>  struct fib_table {
>  	struct hlist_node	tb_hlist;
> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
> index b965f3c0ec9a..fbd029425638 100644
> --- a/net/core/fib_notifier.c
> +++ b/net/core/fib_notifier.c
> @@ -65,8 +65,6 @@ static int fib_net_dump(struct net *net, struct notifier_block *nb)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(ops, &fn_net->fib_notifier_ops, list) {
> -		int err;

Looks like this should have been removed in previous patch

> -
>  		if (!try_module_get(ops->owner))
>  			continue;
>  		err = ops->fib_dump(net, nb);
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index 28cbf07102bc..592d8aef90e3 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -354,15 +354,20 @@ int fib_rules_dump(struct net *net, struct notifier_block *nb, int family)
>  {
>  	struct fib_rules_ops *ops;
>  	struct fib_rule *rule;
> +	int err = 0;
>  
>  	ops = lookup_rules_ops(net, family);
>  	if (!ops)
>  		return -EAFNOSUPPORT;
> -	list_for_each_entry_rcu(rule, &ops->rules_list, list)
> -		call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD, rule, family);
> +	list_for_each_entry_rcu(rule, &ops->rules_list, list) {
> +		err = call_fib_rule_notifier(nb, FIB_EVENT_RULE_ADD,
> +					     rule, family);

Here you add it for rules

> +		if (err)
> +			break;
> +	}
>  	rules_ops_put(ops);
>  
> -	return 0;
> +	return err;
>  }
>  EXPORT_SYMBOL_GPL(fib_rules_dump);