netdev - Re: [patch net-next v2 01/12] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration

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: <YtaX2536pQXe0+f1@shredder>
Date:   Tue, 19 Jul 2022 14:39:07 +0300
From:   Ido Schimmel <idosch@...dia.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        petrm@...dia.com, pabeni@...hat.com, edumazet@...gle.com,
        mlxsw@...dia.com, saeedm@...dia.com, snelson@...sando.io
Subject: Re: [patch net-next v2 01/12] net: devlink: make sure that
 devlink_try_get() works with valid pointer during xarray iteration

> @@ -6511,9 +6566,11 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>  	int err = 0;
>  
>  	mutex_lock(&devlink_mutex);
> +	rcu_read_lock();
>  	xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
>  		if (!devlink_try_get(devlink))
>  			continue;
> +		rcu_read_unlock();
>  
>  		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
>  			goto retry;
> @@ -6537,7 +6594,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
>  		idx++;
>  retry:
>  		devlink_put(devlink);
> +		rcu_read_lock();
>  	}
> +	rcu_read_unlock();
>  	mutex_unlock(&devlink_mutex);
>  
>  	if (err != -EMSGSIZE)

The pw CI is complaining about this:

New errors added
--- /tmp/tmp.Lx7CWX0u9n	2022-07-18 23:56:05.513142294 -0700
+++ /tmp/tmp.SFcuLwts4X	2022-07-18 23:56:49.917188829 -0700
@@ -0,0 +1,2 @@
+../net/core/devlink.c: note: in included file (through ../include/linux/rculist.h, ../include/linux/dcache.h, ../include/linux/fs.h, ../include/linux/highmem.h, ../include/linux/bvec.h, ../include/linux/skbuff.h, ...):
+../include/linux/rcupdate.h:726:9: warning: context imbalance in 'devlink_nl_cmd_info_get_dumpit' - unexpected unlock

Might need something like:

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 0e8274feab11..5a39a02b6ed6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6611,6 +6611,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 			err = 0;
 		else if (err) {
 			devlink_put(devlink);
+			rcu_read_lock();
 			break;
 		}
 inc: