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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Sun, 30 Jul 2023 19:15:58 +0200
From: Simon Horman <horms@...nel.org>
To: Amritha Nambiar <amritha.nambiar@...el.com>
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 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.

> +
> +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;
>  }

...