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

[Greg KH <gregkh@linuxfoundation.org>]

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