netdev - Re: [PATCH net-next v8 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec

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: <Y7668voOCFqWXmdF@hog>
Date:   Wed, 11 Jan 2023 14:34:42 +0100
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     ehakim@...dia.com
Cc:     netdev@...r.kernel.org, raeds@...dia.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        atenart@...nel.org
Subject: Re: [PATCH net-next v8 1/2] macsec: add support for
 IFLA_MACSEC_OFFLOAD in macsec_changelink

Sorry to delay this again, there's something to fix in this version.
I hope you're not too frustrated by the review process.

2023-01-11, 10:11:11 +0200, ehakim@...dia.com wrote:
> +static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
> +	struct nlattr **attrs = info->attrs;
> +	enum macsec_offload offload;
> +	struct macsec_dev *macsec;
> +	struct net_device *dev;
> +	int ret;

        int ret = 0;

Otherwise we can return with ret uninitialized when macsec->offload ==
offload.

(unfortunately the compiler warning is disabled in the kernel Makefile)

[...]
> @@ -3840,8 +3837,17 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (ret)
>  		goto cleanup;
>  
> +	if (data[IFLA_MACSEC_OFFLOAD]) {
> +		offload = nla_get_u8(data[IFLA_MACSEC_OFFLOAD]);
> +		if (macsec->offload != offload) {
> +			macsec_offload_state_change = true;
> +			ret = macsec_update_offload(dev, offload);
> +			if (ret)
> +				goto cleanup;
> +		}
> +	}

nit: there was a blank line here in the previous version, please bring it back.

>  	/* If h/w offloading is available, propagate to the device */
> -	if (macsec_is_offloaded(macsec)) {
> +	if (!macsec_offload_state_change && macsec_is_offloaded(macsec)) {
>  		const struct macsec_ops *ops;
>  		struct macsec_context ctx;

Thanks.

-- 
Sabrina