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: <YtaYVwfAXvEH7bJt@nanopsycho>
Date:   Tue, 19 Jul 2022 13:41:11 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Ido Schimmel <idosch@...dia.com>
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

Tue, Jul 19, 2022 at 01:39:07PM CEST, idosch@...dia.com wrote:
>> @@ -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:

Ah, yes, I missed that. Will add. Thx.


>
>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:
>