netdev - Re: handling MAC set by user space in reset

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YuGFOU7oKlAGZjTa@lunn.ch>
Date:   Wed, 27 Jul 2022 20:34:33 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Oliver Neukum <oneukum@...e.com>
Cc:     Hayes Wang <hayeswang@...ltek.com>,
        USB list <linux-usb@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: handling MAC set by user space in reset_resume() of r8152

On Wed, Jul 27, 2022 at 01:39:43PM +0200, Oliver Neukum wrote:
> Hi,
> 
> looking at the driver it looks to me like the address
> provided to ndo_set_mac_address() is thrown away after use.
> That looks problematic to me, because reset_resume()
> should redo the operation.
> What do you think?
> 
> 	Regards
> 		Oliver

> From 19fc972a5cc98197bc81a7c56dd5d68e3fdfc36b Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oneukum@...e.com>
> Date: Wed, 27 Jul 2022 13:29:42 +0200
> Subject: [PATCH] r8152: restore external MAC in reset_resume
> 
> If user space has set the MAC of the interface,
> reset_resume() must restore that setting rather
> than redetermine the MAC like if te interface
> is probed regularly.
> 
> Signed-off-by: Oliver Neukum <oneukum@...e.com>
> ---
>  drivers/net/usb/r8152.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0f6efaabaa32..5cf74b984655 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -923,6 +923,7 @@ struct r8152 {
>  	atomic_t rx_count;
>  
>  	bool eee_en;
> +	bool external_mac;
>  	int intr_interval;
>  	u32 saved_wolopts;
>  	u32 msg_enable;
> @@ -933,6 +934,8 @@ struct r8152 {
>  	u32 rx_copybreak;
>  	u32 rx_pending;
>  	u32 fc_pause_on, fc_pause_off;
> +	/* for reset_resume */
> +	struct sockaddr saved_addr;
>  
>  	unsigned int pipe_in, pipe_out, pipe_intr, pipe_ctrl_in, pipe_ctrl_out;
>  
> @@ -1574,6 +1577,7 @@ static int __rtl8152_set_mac_address(struct net_device *netdev, void *p,
>  	mutex_lock(&tp->control);
>  
>  	eth_hw_addr_set(netdev, addr->sa_data);
> +	memcpy(&tp->saved_addr, addr, sizeof(tp->saved_addr));

Do you need a copy in tp? I would expect the MAC address stored in
netdev by eth_hw_addr_set() is still there after the resume?

       Andrew