netdev - Re: [PATCH] allow setting wiphy.perm

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: <54DF22CF-59D7-4633-A625-896F58C26A64@holtmann.org>
Date:	Mon, 11 Aug 2014 13:25:40 -0700
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Daniel Gimpelevich <daniel@...pelevich.san-francisco.ca.us>
Cc:	"John W. Linville" <linville@...driver.com>,
	"linux-wireless@...r.kernel.org Wireless" 
	<linux-wireless@...r.kernel.org>,
	Johannes Berg <johannes@...solutions.net>,
	"David S. Miller" <davem@...emloft.net>,
	Network Development <netdev@...r.kernel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] allow setting wiphy.perm_addr after driver probe

Hi Daniel,

> On embedded devices, often the BSSID of an access point must be set from
> userspace during the boot process. This provides a relatively clean way
> of doing that without major side effects.
> 
> Signed-off-by: Daniel Gimpelevich <daniel@...pelevich.san-francisco.ca.us>
> --- 
> --- a/net/wireless/sysfs.c	2014-04-19 08:36:52.048511623 -0700
> +++ b/net/wireless/sysfs.c	2014-04-19 08:38:09.196894176 -0700
> @@ -24,18 +24,30 @@
> 	return container_of(dev, struct cfg80211_registered_device, wiphy.dev);
> }
> 
> -#define SHOW_FMT(name, fmt, member)					\
> +#define SHOW_FMT(name, fmt, member, perm)				\
> static ssize_t name ## _show(struct device *dev,			\
> 			      struct device_attribute *attr,		\
> 			      char *buf)				\
> {									\
> 	return sprintf(buf, fmt "\n", dev_to_rdev(dev)->member);	\
> }									\
> -static DEVICE_ATTR_RO(name)
> +static DEVICE_ATTR_ ## perm(name)
> 
> -SHOW_FMT(index, "%d", wiphy_idx);
> -SHOW_FMT(macaddress, "%pM", wiphy.perm_addr);
> -SHOW_FMT(address_mask, "%pM", wiphy.addr_mask);
> +static ssize_t macaddress_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	u8 temp[ETH_ALEN];
> +
> +	if (sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", temp, temp + 1,
> +			temp + 2, temp + 3, temp + 4, temp + 5) == ETH_ALEN)
> +		memcpy(dev_to_rdev(dev)->wiphy.perm_addr, temp, ETH_ALEN);
> +	return count;
> +}
> +
> +SHOW_FMT(index, "%d", wiphy_idx, RO);
> +SHOW_FMT(macaddress, "%pM", wiphy.perm_addr, RW);
> +SHOW_FMT(address_mask, "%pM", wiphy.addr_mask, RO);

isn't this dangerous to just allow writing to wiphy.perm_addr via sysfs. We ran into the same issue with Bluetooth and ROM only devices that have to unique address. Doing this via sysfs seems the wrong approach. It is messy and full of potential race conditions. I clearly opted against the sysfs solution for Bluetooth. Instead we build an infrastructure that allowed doing it cleanly via the Bluetooth mgmt API. Controllers that have no unique address are brought up as unconfigured and userspace clearly knows that it has to take steps to get an address programmed into the controller.

And I think something similar should be done for WiFi. It might be better to not create the initial wlan0 netdev interface if the hardware has not a single unique address available. That way the supplicant can just either get one from the flash filesystem or make up a proper random one before creating the netdev interface.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html