netdev - Re: [net-next PATCH v1 5/9] netdev-genl: Add netlink framework functions for napi

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: <577e7669-2750-cf08-4382-db865a1c57c1@intel.com>
Date: Mon, 31 Jul 2023 16:00:04 -0700
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Simon Horman <horms@...nel.org>
CC: <netdev@...r.kernel.org>, <kuba@...nel.org>, <davem@...emloft.net>,
	<sridhar.samudrala@...el.com>
Subject: Re: [net-next PATCH v1 5/9] netdev-genl: Add netlink framework
 functions for napi

On 7/30/2023 10:15 AM, Simon Horman wrote:
> On Fri, Jul 28, 2023 at 05:47:17PM -0700, Amritha Nambiar wrote:
>> Implement the netdev netlink framework functions for
>> napi support. The netdev structure tracks all the napi
>> instances and napi fields. The napi instances and associated
>> queue[s] can be retrieved this way.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@...el.com>
>> ---
>>   net/core/netdev-genl.c |  253 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 251 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> 
> ...
> 
>>   int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>>   {
>> -	return -EOPNOTSUPP;
>> +	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
>> +	struct net *net = sock_net(skb->sk);
>> +	struct net_device *netdev;
>> +	int idx = 0, s_idx, n_idx;
>> +	int h, s_h;
>> +	int err;
>> +
>> +	s_h = ctx->dev_entry_hash;
>> +	s_idx = ctx->dev_entry_idx;
>> +	n_idx = ctx->napi_idx;
>> +
>> +	rtnl_lock();
>> +
>> +	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
>> +		struct hlist_head *head;
>> +
>> +		idx = 0;
>> +		head = &net->dev_index_head[h];
>> +		hlist_for_each_entry(netdev, head, index_hlist) {
>> +			if (idx < s_idx)
>> +				goto cont;
>> +			err = netdev_nl_napi_dump_entry(netdev, skb, cb, &n_idx);
>> +			if (err == -EMSGSIZE)
>> +				goto out;
>> +			n_idx = 0;
>> +			if (err < 0)
>> +				break;
>> +cont:
>> +			idx++;
>> +		}
>> +	}
>> +
>> +	rtnl_unlock();
>> +
>> +	return err;
> 
> Hi Amritha,
> 
> I'm unsure if this can happen, but if loop iteration occurs zero times
> above in such a way that netdev_nl_napi_dump_entry() isn't called, then err
> will be uninitialised here.
> 
> This is also the case in netdev_nl_dev_get_dumpit
> (both before and after this patch.
> 
> As flagged by Smatch.
> 

Will fix the initialization in the next version.

>> +
>> +out:
>> +	rtnl_unlock();
>> +
>> +	ctx->dev_entry_idx = idx;
>> +	ctx->dev_entry_hash = h;
>> +	ctx->napi_idx = n_idx;
>> +	cb->seq = net->dev_base_seq;
>> +
>> +	return skb->len;
>>   }
> 
> ...