netdev - Re: [PATCH net-next v6 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: <Y7n6hF3voEe8Hv+5@hog>
Date:   Sun, 8 Jan 2023 00:04:36 +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 v6 1/2] macsec: add support for
 IFLA_MACSEC_OFFLOAD in macsec_changelink

2023-01-06, 15:35:50 +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 net_device *dev;
> +	int ret;
> +
> +	if (!attrs[MACSEC_ATTR_IFINDEX])
> +		return -EINVAL;
> +
> +	if (!attrs[MACSEC_ATTR_OFFLOAD])
> +		return -EINVAL;
> +
> +	if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX,
> +					attrs[MACSEC_ATTR_OFFLOAD],
> +					macsec_genl_offload_policy, NULL))
> +		return -EINVAL;
> +
> +	dev = get_dev_from_nl(genl_info_net(info), attrs);
> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
> +
> +	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
> +		return -EINVAL;
> +
> +	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> +
> +	rtnl_lock();

Why are you putting rtnl_lock() back down here? You just moved it
above get_dev_from_nl with commit f3b4a00f0f62 ("net: macsec: fix net
device access prior to holding a lock"), now you're pretty much
reverting that fix.

> +
> +	ret = macsec_update_offload(dev, offload);
>  
> -rollback:
> -	macsec->offload = prev_offload;
> -out:
>  	rtnl_unlock();
> +
>  	return ret;
>  }
>  

-- 
Sabrina