Re: [PATCH v3 4/5] mac80211_hwsim: add permanent mac address option for new radios

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> 
>  
> +	if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
> +		if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) != ETH_ALEN) {
> +			pr_debug("mac80211_hwsim: MAC has wrong size\n");
> +			return -EINVAL;
> +		}

Oh, also - don't do this.

1) don't print, use NL_SET_ERR_MSG().

2) use the policy to validate lengths

> +		if (!is_local_ether_addr(
> +				nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> +			pr_debug("mac80211_hwsim: MAC is not locally administered\n");
> +			return -EINVAL;
> +		}

This doesn't really matter - it's purely virtual. I think we can avoid
that.

> +		if (get_hwsim_data_ref_from_addr(
> +				nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> +			pr_debug("mac80211_hwsim: perm MAC already in use\n");
> +			return -EINVAL;
> +		}

This is racy afaict - remove it and return a clash later when you fail
to insert the new radio.

johannes