netdev - Re: [PATCH v3] r8152: Add support for setting pass through MAC address on RTL8153-AD

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]

Date:	Mon, 6 Jun 2016 07:39:47 -0700
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Mario Limonciello <mario_limonciello@...l.com>
Cc:	hayeswang@...ltek.com, LKML <linux-kernel@...r.kernel.org>,
	Netdev <netdev@...r.kernel.org>,
	Linux USB <linux-usb@...r.kernel.org>, pali.rohar@...il.com,
	anthony.wong@...onical.com
Subject: Re: [PATCH v3] r8152: Add support for setting pass through MAC
 address on RTL8153-AD

On Mon, Jun 06, 2016 at 09:15:21AM -0500, Mario Limonciello wrote:
> The RTL8153-AD supports a persistent system specific MAC address.
> This means a device plugged into two different systems with host side
> support will show different (but persistent) MAC addresses.
> 
> This information for the system's persistent MAC address is burned in when
> the system HW is built and available under _SB\AMAC in the DSDT at runtime.
> 
> This technology is currently implemented in the Dell TB15 and WD15 Type-C
> docks.  More information is available here:
> http://www.dell.com/support/article/us/en/04/SLN301147
> 
> Signed-off-by: Mario Limonciello <mario_limonciello@...l.com>
> ---
>  drivers/net/usb/r8152.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 3f9f6ed..f4bd46d 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -26,6 +26,7 @@
>  #include <linux/mdio.h>
>  #include <linux/usb/cdc.h>
>  #include <linux/suspend.h>
> +#include <linux/acpi.h>
>  
>  /* Information for net-next */
>  #define NETNEXT_VERSION		"08"
> @@ -455,6 +456,11 @@
>  /* SRAM_IMPEDANCE */
>  #define RX_DRIVING_MASK		0x6000
>  
> +/* MAC PASSTHRU */
> +#define AD_MASK			0xfee0
> +#define EFUSE			0xcfdb
> +#define PASS_THRU_MASK		0x1
> +
>  enum rtl_register_content {
>  	_1000bps	= 0x10,
>  	_100bps		= 0x08,
> @@ -1030,6 +1036,53 @@ out1:
>  	return ret;
>  }
>  
> +static int get_passthru_addr(struct r8152 *tp, struct sockaddr *sa)

I see no "Dell" specific markings or tests here at all, please be
EXPLICIT in the code exactly what this is for, and what it is doing.
Otherwise no one can tell you what machines/devices this will trigger on
at all.

As it is, it's totally vague and unknown what this whole function is
here for.



> +{
> +	acpi_status status;
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret = -EINVAL;
> +	u32 ocp_data;
> +	unsigned char buf[6];
> +
> +	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
> +	if ((ocp_data & AD_MASK) != 0x1000)
> +		return -ENODEV;
> +
> +	ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE);
> +	if ((ocp_data & PASS_THRU_MASK) != 1)
> +		return -ENODEV;
> +
> +	/* returns _AUXMAC_#AABBCCDDEEFF# */
> +	status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
> +	obj = (union acpi_object *)buffer.pointer;
> +	if (ACPI_SUCCESS(status)) {
> +		if (obj->type != ACPI_TYPE_BUFFER ||
> +		    obj->string.length != 0x17) {
> +			pr_warn("r8152: get_passthru_addr: Invalid buffer");
> +			goto amacout;
> +		}
> +		if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0) {
> +			pr_warn("r8152: get_passthru_addr: Invalid header");
> +			goto amacout;
> +		}
> +		ret = hex2bin(buf, obj->string.pointer + 9, 6);
> +		if (ret < 0 || !is_valid_ether_addr(buf)) {
> +			pr_warn("r8152: get_passthru_addr: Invalid MAC");
> +			goto amacout;
> +		}
> +		memcpy(sa->sa_data, buf, 6);
> +		ether_addr_copy(tp->netdev->dev_addr, sa->sa_data);
> +		netdev_info(tp->netdev, "Using pass-through MAC address %pM\n",
> +			    sa->sa_data);
> +		ret = 0;
> +	}
> +
> +amacout:
> +	kfree(obj);
> +	return ret;
> +}
> +
>  static int set_ethernet_addr(struct r8152 *tp)
>  {
>  	struct net_device *dev = tp->netdev;
> @@ -1038,8 +1091,11 @@ static int set_ethernet_addr(struct r8152 *tp)
>  
>  	if (tp->version == RTL_VER_01)
>  		ret = pla_ocp_read(tp, PLA_IDR, 8, sa.sa_data);
> -	else
> -		ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
> +	else {
> +		ret = get_passthru_addr(tp, &sa);
> +		if (ret < 0)

an "error" here is a "normal" thing, so please document it.

Or again, change the function to be "dell_crazy_hardware_hack_enabled()"
so it'b obvious what is going on.

greg k-h