netdev - Re: [PATCH net-next] net: dcb: move getapptrust to separate function

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: <87eduaoj8g.fsf@nvidia.com>
Date:   Thu, 10 Nov 2022 17:30:43 +0100
From:   Petr Machata <petrm@...dia.com>
To:     Daniel Machon <daniel.machon@...rochip.com>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <joe@...ches.com>, <vladimir.oltean@....com>, <petrm@...dia.com>,
        <linux-kernel@...r.kernel.org>, <UNGLinuxDriver@...rochip.com>,
        "kernel test robot" <lkp@...el.com>
Subject: Re: [PATCH net-next] net: dcb: move getapptrust to separate function


Daniel Machon <daniel.machon@...rochip.com> writes:

> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index cec0632f96db..3f4d88c1ec78 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -1060,11 +1060,52 @@ static int dcbnl_build_peer_app(struct net_device *netdev, struct sk_buff* skb,
>  	return err;
>  }
>  
> +static int dcbnl_getapptrust(struct net_device *netdev, struct sk_buff *skb)
> +{
> +	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> +	int nselectors, err;
> +	u8 *selectors;
> +
> +	selectors = kzalloc(IEEE_8021QAZ_APP_SEL_MAX + 1, GFP_KERNEL);
> +	if (!selectors)
> +		return -ENOMEM;
> +
> +	err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
> +
> +	if (!err) {
> +		struct nlattr *apptrust;
> +		int i;

(Maybe consider moving these up to the function scope. This scope
business made sense in the generic function, IMHO is not as useful with
a focused function like this one.)

> +
> +		err = -EMSGSIZE;
> +
> +		apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
> +		if (!apptrust)
> +			goto nla_put_failure;
> +
> +		for (i = 0; i < nselectors; i++) {
> +			enum ieee_attrs_app type =
> +				dcbnl_app_attr_type_get(selectors[i]);

Doesn't checkpatch warn about this? There should be a blank line after
the variable declaration block. (Even if there wasn't one there in the
original code either.)

> +			err = nla_put_u8(skb, type, selectors[i]);
> +			if (err) {
> +				nla_nest_cancel(skb, apptrust);
> +				goto nla_put_failure;
> +			}
> +		}
> +		nla_nest_end(skb, apptrust);
> +	}
> +
> +	err = 0;
> +
> +nla_put_failure:
> +	kfree(selectors);
> +	return err;
> +}
> +