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: <CAEyMn7aZy+RAo3m=idPi0P152UO+WPm0sfQFdDrUpwmeuUgYBQ@mail.gmail.com>
Date: Tue, 23 Apr 2024 10:00:55 +0200
From: Heiko Thiery <heiko.thiery@...il.com>
To: Alexis Lothoré <alexis.lothore@...tlin.com>
Cc: Ajay Singh <ajay.kathat@...rochip.com>, Claudiu Beznea <claudiu.beznea@...on.dev>, 
	Kalle Valo <kvalo@...nel.org>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/6] wifi: wilc1000: read MAC address from fuse at probe

Hi Alexis, All

Am Mi., 17. Apr. 2024 um 11:34 Uhr schrieb Alexis Lothoré
<alexis.lothore@...tlin.com>:
>
> From: Ajay Singh <ajay.kathat@...rochip.com>
>
> The default netdev interface exposed by WILC1000 is registered at probe,
> but the chip mac address is not known until ndo_open, which will load and
> start chip firmware and then retrieve stored MAC address from it. As a
> consequence, the interface has uninitialized value (00:00:00:00:00) until a
> user brings up the interface.
>
> Fix MAC address at probe by setting the following steps:
> - at probe, read MAC address directly from fuse
> - whenever a new netdevice is created, apply saved mac address (which can
>   be a user-provided address, or the eFuse Mac address if no address has
>   been passed by user)
> - whenever an interface is brought up for the first time (and so the
>   firmware is loaded and started), enforce netdevice mac address to the
>   chip (in case user has changed it)
>
> Reported-by: Heiko Thiery <heiko.thiery@...il.com>
> Closes: https://lore.kernel.org/netdev/CAEyMn7aV-B4OEhHR4Ad0LM3sKCz1-nDqSb9uZNmRWR-hMZ=z+A@mail.gmail.com/T/
> Signed-off-by: Ajay Singh <ajay.kathat@...rochip.com>
> Co-developed-by: Alexis Lothoré <alexis.lothore@...tlin.com>
> Signed-off-by: Alexis Lothoré <alexis.lothore@...tlin.com>

I applied the series on next and tested it on my env. It looks good.

Tested-by: Heiko Thiery <heiko.thiery@...il.com>

Thank you!
Heiko

> ---
>  drivers/net/wireless/microchip/wilc1000/netdev.c | 38 ++++++++++++++----------
>  drivers/net/wireless/microchip/wilc1000/sdio.c   | 14 +++++++++
>  drivers/net/wireless/microchip/wilc1000/spi.c    |  6 ++++
>  3 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
> index 5ab448d0b643..f1fbc6e8a82a 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.c
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
> @@ -588,7 +588,6 @@ static int wilc_mac_open(struct net_device *ndev)
>         struct wilc *wl = vif->wilc;
>         int ret = 0;
>         struct mgmt_frame_regs mgmt_regs = {};
> -       u8 addr[ETH_ALEN] __aligned(2);
>
>         if (!wl || !wl->dev) {
>                 netdev_err(ndev, "device not ready\n");
> @@ -607,25 +606,19 @@ static int wilc_mac_open(struct net_device *ndev)
>                 return ret;
>         }
>
> -       wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
> -                               vif->idx);
> -
> -       if (is_valid_ether_addr(ndev->dev_addr)) {
> -               ether_addr_copy(addr, ndev->dev_addr);
> -               wilc_set_mac_address(vif, addr);
> -       } else {
> -               wilc_get_mac_address(vif, addr);
> -               eth_hw_addr_set(ndev, addr);
> -       }
>         netdev_dbg(ndev, "Mac address: %pM\n", ndev->dev_addr);
> -
> -       if (!is_valid_ether_addr(ndev->dev_addr)) {
> -               netdev_err(ndev, "Wrong MAC address\n");
> +       ret = wilc_set_mac_address(vif, ndev->dev_addr);
> +       if (ret) {
> +               netdev_err(ndev, "Failed to enforce MAC address in chip");
>                 wilc_deinit_host_int(ndev);
> -               wilc_wlan_deinitialize(ndev);
> -               return -EINVAL;
> +               if (!wl->open_ifcs)
> +                       wilc_wlan_deinitialize(ndev);
> +               return ret;
>         }
>
> +       wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
> +                               vif->idx);
> +
>         mgmt_regs.interface_stypes = vif->mgmt_reg_stypes;
>         /* so we detect a change */
>         vif->mgmt_reg_stypes = 0;
> @@ -941,6 +934,7 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
>                                       int vif_type, enum nl80211_iftype type,
>                                       bool rtnl_locked)
>  {
> +       u8 mac_address[ETH_ALEN];
>         struct net_device *ndev;
>         struct wilc_vif *vif;
>         int ret;
> @@ -969,6 +963,18 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
>         vif->iftype = vif_type;
>         vif->idx = wilc_get_available_idx(wl);
>         vif->mac_opened = 0;
> +
> +       memcpy(mac_address, wl->nv_mac_address, ETH_ALEN);
> +       /* WILC firmware uses locally administered MAC address for the
> +        * second virtual interface (bit 1 of first byte set), but
> +        * since it is possibly not loaded/running yet, reproduce this behavior
> +        * in the driver during interface creation.
> +        */
> +       if (vif->idx)
> +               mac_address[0] |= 0x2;
> +
> +       eth_hw_addr_set(vif->ndev, mac_address);
> +
>         mutex_lock(&wl->vif_mutex);
>         list_add_tail_rcu(&vif->list, &wl->vif_list);
>         wl->vif_num += 1;
> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
> index 04d6565df2cb..e6e20c86b791 100644
> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> @@ -24,6 +24,9 @@ MODULE_DEVICE_TABLE(sdio, wilc_sdio_ids);
>
>  #define WILC_SDIO_BLOCK_SIZE 512
>
> +static int wilc_sdio_init(struct wilc *wilc, bool resume);
> +static int wilc_sdio_deinit(struct wilc *wilc);
> +
>  struct wilc_sdio {
>         bool irq_gpio;
>         u32 block_size;
> @@ -178,6 +181,16 @@ static int wilc_sdio_probe(struct sdio_func *func,
>         }
>         clk_prepare_enable(wilc->rtc_clk);
>
> +       wilc_sdio_init(wilc, false);
> +
> +       ret = wilc_load_mac_from_nv(wilc);
> +       if (ret) {
> +               pr_err("Can not retrieve MAC address from chip\n");
> +               goto clk_disable_unprepare;
> +       }
> +
> +       wilc_sdio_deinit(wilc);
> +
>         vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
>                                    NL80211_IFTYPE_STATION, false);
>         if (IS_ERR(vif)) {
> @@ -187,6 +200,7 @@ static int wilc_sdio_probe(struct sdio_func *func,
>
>         dev_info(&func->dev, "Driver Initializing success\n");
>         return 0;
> +
>  clk_disable_unprepare:
>         clk_disable_unprepare(wilc->rtc_clk);
>  dispose_irq:
> diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
> index add0e70a09ea..5ff940c53ad9 100644
> --- a/drivers/net/wireless/microchip/wilc1000/spi.c
> +++ b/drivers/net/wireless/microchip/wilc1000/spi.c
> @@ -250,6 +250,12 @@ static int wilc_bus_probe(struct spi_device *spi)
>         if (ret)
>                 goto power_down;
>
> +       ret = wilc_load_mac_from_nv(wilc);
> +       if (ret) {
> +               pr_err("Can not retrieve MAC address from chip\n");
> +               goto power_down;
> +       }
> +
>         wilc_wlan_power(wilc, false);
>         vif = wilc_netdev_ifc_init(wilc, "wlan%d", WILC_STATION_MODE,
>                                    NL80211_IFTYPE_STATION, false);
>
> --
> 2.44.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ