[PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[Franklin Lin]

From: franklin_lin <franklin_lin@wistron.corp-partner.google.com>

Enable the support for providing a MAC address
for a dock to use based on the VPD values set in the platform.

Signed-off-by: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
---
 drivers/net/usb/r8152.c | 49 ++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7389d6ef8..732e48d99 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3,6 +3,7 @@
  *  Copyright (c) 2014 Realtek Semiconductor Corp. All rights reserved.
  */
 
+#include <linux/fs.h>
 #include <linux/signal.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -1608,6 +1609,11 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
 	acpi_object_type mac_obj_type;
 	int mac_strlen;
 
+	struct file *fp;
+	unsigned char read_buf[32];
+	loff_t f_pos = 0;
+	int i, j, len;
+
 	if (tp->lenovo_macpassthru) {
 		mac_obj_name = "\\MACA";
 		mac_obj_type = ACPI_TYPE_STRING;
@@ -1641,22 +1647,39 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
 	/* returns _AUXMAC_#AABBCCDDEEFF# */
 	status = acpi_evaluate_object(NULL, mac_obj_name, NULL, &buffer);
 	obj = (union acpi_object *)buffer.pointer;
-	if (!ACPI_SUCCESS(status))
-		return -ENODEV;
-	if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
-		netif_warn(tp, probe, tp->netdev,
+	if (ACPI_SUCCESS(status)) {
+		if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
+			netif_warn(tp, probe, tp->netdev,
 			   "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
 			   obj->type, obj->string.length);
-		goto amacout;
-	}
-
-	if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
-	    strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
-		netif_warn(tp, probe, tp->netdev,
-			   "Invalid header when reading pass-thru MAC addr\n");
-		goto amacout;
+			goto amacout;
+		}
+		if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
+			strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
+			netif_warn(tp, probe, tp->netdev,
+				"Invalid header when reading pass-thru MAC addr\n");
+			goto amacout;
+		}
+		ret = hex2bin(buf, obj->string.pointer + 9, 6);
+	} else {
+		/* read from "/sys/firmware/vpd/ro/dock_mac" */
+		fp = filp_open("/sys/firmware/vpd/ro/dock_mac", O_RDONLY, 0);
+		if (IS_ERR(fp))
+			return -ENOENT;
+		kernel_read(fp, read_buf, 32, &f_pos);
+		len = strlen(read_buf);
+		/* remove ':' form mac address string */
+		for (i = 0; i < len; i++) {
+			if (read_buf[i] == ':') {
+				for (j = i; j < len; j++)
+					read_buf[j] = read_buf[j+1];
+				len--;
+				i--;
+			}
+		}
+		filp_close(fp, NULL);
+		ret = hex2bin(buf, read_buf, 6);
 	}
-	ret = hex2bin(buf, obj->string.pointer + 9, 6);
 	if (!(ret == 0 && is_valid_ether_addr(buf))) {
 		netif_warn(tp, probe, tp->netdev,
 			   "Invalid MAC for pass-thru MAC addr: %d, %pM\n",
-- 
2.34.1

Re: [PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[Greg KH]

On Tue, Jun 28, 2022 at 09:53:25AM +0800, Franklin Lin wrote:
> From: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
> 
> Enable the support for providing a MAC address
> for a dock to use based on the VPD values set in the platform.
> 
> Signed-off-by: franklin_lin <franklin_lin@wistron.corp-partner.google.com>

Please use your name "Franklin Lin", and the corp-partner.google.com
email addresses are not "real" addresses.  Please use your normal
corporate one instead.

thanks,

greg k-h

Re: [PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[Greg KH]

On Tue, Jun 28, 2022 at 09:53:25AM +0800, Franklin Lin wrote:
> From: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
> 
> Enable the support for providing a MAC address
> for a dock to use based on the VPD values set in the platform.
> 
> Signed-off-by: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
> ---
>  drivers/net/usb/r8152.c | 49 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 7389d6ef8..732e48d99 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -3,6 +3,7 @@
>   *  Copyright (c) 2014 Realtek Semiconductor Corp. All rights reserved.
>   */
>  
> +#include <linux/fs.h>
>  #include <linux/signal.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> @@ -1608,6 +1609,11 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
>  	acpi_object_type mac_obj_type;
>  	int mac_strlen;
>  
> +	struct file *fp;
> +	unsigned char read_buf[32];
> +	loff_t f_pos = 0;
> +	int i, j, len;
> +
>  	if (tp->lenovo_macpassthru) {
>  		mac_obj_name = "\\MACA";
>  		mac_obj_type = ACPI_TYPE_STRING;
> @@ -1641,22 +1647,39 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
>  	/* returns _AUXMAC_#AABBCCDDEEFF# */
>  	status = acpi_evaluate_object(NULL, mac_obj_name, NULL, &buffer);
>  	obj = (union acpi_object *)buffer.pointer;
> -	if (!ACPI_SUCCESS(status))
> -		return -ENODEV;
> -	if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
> -		netif_warn(tp, probe, tp->netdev,
> +	if (ACPI_SUCCESS(status)) {
> +		if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
> +			netif_warn(tp, probe, tp->netdev,
>  			   "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
>  			   obj->type, obj->string.length);
> -		goto amacout;
> -	}
> -
> -	if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
> -	    strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
> -		netif_warn(tp, probe, tp->netdev,
> -			   "Invalid header when reading pass-thru MAC addr\n");
> -		goto amacout;
> +			goto amacout;
> +		}
> +		if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
> +			strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
> +			netif_warn(tp, probe, tp->netdev,
> +				"Invalid header when reading pass-thru MAC addr\n");
> +			goto amacout;
> +		}
> +		ret = hex2bin(buf, obj->string.pointer + 9, 6);
> +	} else {
> +		/* read from "/sys/firmware/vpd/ro/dock_mac" */
> +		fp = filp_open("/sys/firmware/vpd/ro/dock_mac", O_RDONLY, 0);

Woah, what?  No, that's not how firmware works at all, sorry.  Please
use the correct firmware interface if this really is firmware.  If it is
not, please use the correct networking interface instead.

you should NEVER read from a file from a driver, that is a sure sign
something is really really wrong.

thanks,

greg k-h

Re: [PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[Andrew Lunn]

On Tue, Jun 28, 2022 at 09:53:25AM +0800, Franklin Lin wrote:
> From: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
> 
> Enable the support for providing a MAC address
> for a dock to use based on the VPD values set in the platform.

Maybe i'm missing it, but i don't see any code which verifies this
r8152 is actually in the dock, and not a USB ethernet dongle plugged
into some port of either the laptop or the dock itself.

     Andrew

Re: [PATCH] drivers/net/usb/r8152: Enable MAC address passthru support

[Oliver Neukum]

On 28.06.22 12:17, Andrew Lunn wrote:
> On Tue, Jun 28, 2022 at 09:53:25AM +0800, Franklin Lin wrote:
>> From: franklin_lin <franklin_lin@wistron.corp-partner.google.com>
>>
>> Enable the support for providing a MAC address
>> for a dock to use based on the VPD values set in the platform.
> 
> Maybe i'm missing it, but i don't see any code which verifies this
> r8152 is actually in the dock, and not a USB ethernet dongle plugged
> into some port of either the laptop or the dock itself.

And it will happily assign the same MAC to multiple devices.
I am afraid this patch needs some basic redesign.

	Regards
		Oliver