[bug] spi-nor not unlocking on Ubiquiti XW and WA

[bug] spi-nor not unlocking on Ubiquiti XW and WA

[Jean-Marc Ranger]

Hello,

I'm reporting this as an user of OpenWRT 23.05 on an Ubiquiti 
Nanostation AC, who hopes to upgrade to 24.10. Currently, upgrading 
results in a device where no configuration can be saved. Recovery 
requires physical presence, on devices often installed outdoor.

The issue has an explanation and a patch (for kernel 6.6) has been 
available since late 2024 [1, attached below][2, which merges multiple 
fixes]. It has been used by others [3] and myself. However, its author 
believes that it doesn't have the correct approach to be upstreamed [4]. 
OpenWRT maintainers are waiting for an ACK or better from upstream 
before applying [5].

Is there a way to "unlock" this (pun intended) ?

For reference, OpenWRT configures those devices with 
CONFIG_MTD_SPI_NOR_SWP_DISABLE [6].

I'm available to test, but:
- the only device I can use is a production one
- OpenWRT is still working on upgrading ath79 from 6.6 to 6.12 [7], so 
I'd be limited to testing on 6.6

Let me know how I can help.

Thanks!

Jean-Marc


[1] 
https://github.com/openwrt/openwrt/pull/17287/commits/c98a55f95268f109911c5fddf5a153cfe3565b74
[2] https://github.com/aredn/aredn/blob/main/patches/006-flash-fixes.patch
[3] https://github.com/openwrt/openwrt/issues/17285#issuecomment-2946978832
[4] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558569582
[5] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558502454
[6] 
https://github.com/openwrt/openwrt/blob/bb59922007043c0a0813d62b0d9f6e801caff9df/target/linux/ath79/generic/config-default
[7] https://github.com/openwrt/openwrt/issues/16569


 From c98a55f95268f109911c5fddf5a153cfe3565b74 Mon Sep 17 00:00:00 2001
From: Tim Wilkinson <tim.j.wilkinson@gmail.com>
Date: Mon, 16 Dec 2024 09:37:34 -0800
Subject: [PATCH] kernel: Fix setup of flash chips which must be unlocked
  before use.

Setup the mtd information for spi nor flash before calling running
the nor setup.

The current code assumes the nor setup doesnt make reference to the mtd
information. There's even a comment to this effect. However, at least
when flash chips must be unlocked, this isn't true.  Consequently, the
unlock code will fail because it makes reference to uninitialized mtd
information. This failure is silent because the bad mtd information
claims the flash is 0 bytes long, and so there is nothing to unlock.

This patch moves the mtd initialization before the nor setup, so the
mtd info is available.

This fix has been tested on two different Ubiquiti devices - the
Rocket AC Lite and the Rocket M5 XW. These use the mx25l12805d and
mx25l6405d Macronix flash chips respectively.  Previously these devices
had failed to operate correctly as it was not possible for any
persistent changes to be made once the factory build had been installed.
With this change these devices behave correctly.

Signed-off-by: Tim Wilkinson <tim.j.wilkinson@gmail.com>
---
  .../436-mtd-spi-earlier-mtd-setup.patch       | 20 +++++++++++++++++++
  1 file changed, 20 insertions(+)
  create mode 100644 
target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch

diff --git 
a/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch 
b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch
new file mode 100644
index 00000000000000..da75e9f7abfe96
--- /dev/null
+++ b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch
@@ -0,0 +1,20 @@
+--- a/drivers/mtd/spi-nor/core.c
++++ b/drivers/mtd/spi-nor/core.c
+@@ -3540,14 +3540,14 @@
+ 	if (ret)
+ 		return ret;
+
++	/* No mtd_info fields should be used up to this point. */
++	spi_nor_set_mtd_info(nor);
++
+ 	/* Send all the required SPI flash commands to initialize device */
+ 	ret = spi_nor_init(nor);
+ 	if (ret)
+ 		return ret;
+
+-	/* No mtd_info fields should be used up to this point. */
+-	spi_nor_set_mtd_info(nor);
+-
+ 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
+ 			(long long)mtd->size >> 10);
+

Re: [bug] spi-nor not unlocking on Ubiquiti XW and WA

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 7566 bytes --]

Hi,

> I'm reporting this as an user of OpenWRT 23.05 on an Ubiquiti 
> Nanostation AC, who hopes to upgrade to 24.10. Currently, upgrading 
> results in a device where no configuration can be saved. Recovery 
> requires physical presence, on devices often installed outdoor.

thanks for the bug report.

> The issue has an explanation and a patch (for kernel 6.6) has been 
> available since late 2024 [1, attached below][2, which merges multiple 
> fixes]. It has been used by others [3] and myself. However, its author 
> believes that it doesn't have the correct approach to be upstreamed [4]. 
> OpenWRT maintainers are waiting for an ACK or better from upstream 
> before applying [5].
>
> Is there a way to "unlock" this (pun intended) ?
>
> For reference, OpenWRT configures those devices with 
> CONFIG_MTD_SPI_NOR_SWP_DISABLE [6].
>
> I'm available to test, but:
> - the only device I can use is a production one
> - OpenWRT is still working on upgrading ath79 from 6.6 to 6.12 [7], so 
> I'd be limited to testing on 6.6
>
> Let me know how I can help.
>
> Thanks!
>
> Jean-Marc
>
>
> [1] 
> https://github.com/openwrt/openwrt/pull/17287/commits/c98a55f95268f109911c5fddf5a153cfe3565b74
> [2] https://github.com/aredn/aredn/blob/main/patches/006-flash-fixes.patch
> [3] https://github.com/openwrt/openwrt/issues/17285#issuecomment-2946978832
> [4] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558569582
> [5] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558502454
> [6] 
> https://github.com/openwrt/openwrt/blob/bb59922007043c0a0813d62b0d9f6e801caff9df/target/linux/ath79/generic/config-default
> [7] https://github.com/openwrt/openwrt/issues/16569
>
>
>  From c98a55f95268f109911c5fddf5a153cfe3565b74 Mon Sep 17 00:00:00 2001
> From: Tim Wilkinson <tim.j.wilkinson@gmail.com>
> Date: Mon, 16 Dec 2024 09:37:34 -0800
> Subject: [PATCH] kernel: Fix setup of flash chips which must be unlocked
>   before use.
>
> Setup the mtd information for spi nor flash before calling running
> the nor setup.
>
> The current code assumes the nor setup doesnt make reference to the mtd
> information. There's even a comment to this effect. However, at least
> when flash chips must be unlocked, this isn't true.  Consequently, the
> unlock code will fail because it makes reference to uninitialized mtd
> information. This failure is silent because the bad mtd information
> claims the flash is 0 bytes long, and so there is nothing to unlock.
>
> This patch moves the mtd initialization before the nor setup, so the
> mtd info is available.
>
> This fix has been tested on two different Ubiquiti devices - the
> Rocket AC Lite and the Rocket M5 XW. These use the mx25l12805d and
> mx25l6405d Macronix flash chips respectively.  Previously these devices
> had failed to operate correctly as it was not possible for any
> persistent changes to be made once the factory build had been installed.
> With this change these devices behave correctly.
>
> Signed-off-by: Tim Wilkinson <tim.j.wilkinson@gmail.com>
> ---
>   .../436-mtd-spi-earlier-mtd-setup.patch       | 20 +++++++++++++++++++
>   1 file changed, 20 insertions(+)
>   create mode 100644 
> target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch
>
> diff --git 
> a/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch 
> b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch
> new file mode 100644
> index 00000000000000..da75e9f7abfe96
> --- /dev/null
> +++ b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch
> @@ -0,0 +1,20 @@
> +--- a/drivers/mtd/spi-nor/core.c
> ++++ b/drivers/mtd/spi-nor/core.c
> +@@ -3540,14 +3540,14 @@
> + 	if (ret)
> + 		return ret;
> +
> ++	/* No mtd_info fields should be used up to this point. */
> ++	spi_nor_set_mtd_info(nor);
> ++
> + 	/* Send all the required SPI flash commands to initialize device */
> + 	ret = spi_nor_init(nor);
> + 	if (ret)
> + 		return ret;
> +
> +-	/* No mtd_info fields should be used up to this point. */
> +-	spi_nor_set_mtd_info(nor);
> +-

This seems to be due to the use of the uninitalized "mtd->size".
Could you try the following patch which is based on the latest
next kernel. It replaces mtd->size with nor->params->size, so you
could backport it to 6.6, but maybe it will apply anyway.

---snip---
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 9c9328478d8a..9b07f83aeac7 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -56,7 +56,6 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
 static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
 					u64 *len)
 {
-	struct mtd_info *mtd = &nor->mtd;
 	u64 min_prot_len;
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
@@ -77,13 +76,13 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
 	min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 	*len = min_prot_len << (bp - 1);
 
-	if (*len > mtd->size)
-		*len = mtd->size;
+	if (*len > nor->params->size)
+		*len = nor->params->size;
 
 	if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask)
 		*ofs = 0;
 	else
-		*ofs = mtd->size - *len;
+		*ofs = nor->params->size - *len;
 }
 
 /*
@@ -158,7 +157,6 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
  */
 static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
-	struct mtd_info *mtd = &nor->mtd;
 	u64 min_prot_len;
 	int ret, status_old, status_new;
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
@@ -183,7 +181,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 		can_be_bottom = false;
 
 	/* If anything above us is unlocked, we can't use 'top' protection */
-	if (!spi_nor_is_locked_sr(nor, ofs + len, mtd->size - (ofs + len),
+	if (!spi_nor_is_locked_sr(nor, ofs + len, nor->params->size - (ofs + len),
 				  status_old))
 		can_be_top = false;
 
@@ -195,11 +193,11 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 
 	/* lock_len: length of region that should end up locked */
 	if (use_top)
-		lock_len = mtd->size - ofs;
+		lock_len = nor->params->size - ofs;
 	else
 		lock_len = ofs + len;
 
-	if (lock_len == mtd->size) {
+	if (lock_len == nor->params->size) {
 		val = mask;
 	} else {
 		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
@@ -248,7 +246,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
  */
 static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
-	struct mtd_info *mtd = &nor->mtd;
 	u64 min_prot_len;
 	int ret, status_old, status_new;
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
@@ -273,7 +270,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 		can_be_top = false;
 
 	/* If anything above us is locked, we can't use 'bottom' protection */
-	if (!spi_nor_is_unlocked_sr(nor, ofs + len, mtd->size - (ofs + len),
+	if (!spi_nor_is_unlocked_sr(nor, ofs + len, nor->params->size - (ofs + len),
 				    status_old))
 		can_be_bottom = false;
 
@@ -285,7 +282,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 
 	/* lock_len: length of region that should remain locked */
 	if (use_top)
-		lock_len = mtd->size - (ofs + len);
+		lock_len = nor->params->size - (ofs + len);
 	else
 		lock_len = ofs;
 
---snip---

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [bug] spi-nor not unlocking on Ubiquiti XW and WA

[Jean-Marc Ranger]

On 2025-06-30 05:25, Michael Walle wrote:
> This seems to be due to the use of the uninitalized "mtd->size".
> Could you try the following patch which is based on the latest
> next kernel. It replaces mtd->size with nor->params->size, so you
> could backport it to 6.6, but maybe it will apply anyway.

Thank you so much for taking the time!

Your patch applies automatically on 6.6.93, with minimal fuzzing and 
offset changes. And it fixes the issue! Formally:
Tested-by: Jean-Marc Ranger <jmranger@hotmail.com>

Thanks again,

Jean-Marc

> ---snip---
> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
> index 9c9328478d8a..9b07f83aeac7 100644
> --- a/drivers/mtd/spi-nor/swp.c
> +++ b/drivers/mtd/spi-nor/swp.c
> @@ -56,7 +56,6 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
>   static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
>   					u64 *len)
>   {
> -	struct mtd_info *mtd = &nor->mtd;
>   	u64 min_prot_len;
>   	u8 mask = spi_nor_get_sr_bp_mask(nor);
>   	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
> @@ -77,13 +76,13 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
>   	min_prot_len = spi_nor_get_min_prot_length_sr(nor);
>   	*len = min_prot_len << (bp - 1);
>   
> -	if (*len > mtd->size)
> -		*len = mtd->size;
> +	if (*len > nor->params->size)
> +		*len = nor->params->size;
>   
>   	if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask)
>   		*ofs = 0;
>   	else
> -		*ofs = mtd->size - *len;
> +		*ofs = nor->params->size - *len;
>   }
>   
>   /*
> @@ -158,7 +157,6 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
>    */
>   static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>   {
> -	struct mtd_info *mtd = &nor->mtd;
>   	u64 min_prot_len;
>   	int ret, status_old, status_new;
>   	u8 mask = spi_nor_get_sr_bp_mask(nor);
> @@ -183,7 +181,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>   		can_be_bottom = false;
>   
>   	/* If anything above us is unlocked, we can't use 'top' protection */
> -	if (!spi_nor_is_locked_sr(nor, ofs + len, mtd->size - (ofs + len),
> +	if (!spi_nor_is_locked_sr(nor, ofs + len, nor->params->size - (ofs + len),
>   				  status_old))
>   		can_be_top = false;
>   
> @@ -195,11 +193,11 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>   
>   	/* lock_len: length of region that should end up locked */
>   	if (use_top)
> -		lock_len = mtd->size - ofs;
> +		lock_len = nor->params->size - ofs;
>   	else
>   		lock_len = ofs + len;
>   
> -	if (lock_len == mtd->size) {
> +	if (lock_len == nor->params->size) {
>   		val = mask;
>   	} else {
>   		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
> @@ -248,7 +246,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>    */
>   static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
>   {
> -	struct mtd_info *mtd = &nor->mtd;
>   	u64 min_prot_len;
>   	int ret, status_old, status_new;
>   	u8 mask = spi_nor_get_sr_bp_mask(nor);
> @@ -273,7 +270,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
>   		can_be_top = false;
>   
>   	/* If anything above us is locked, we can't use 'bottom' protection */
> -	if (!spi_nor_is_unlocked_sr(nor, ofs + len, mtd->size - (ofs + len),
> +	if (!spi_nor_is_unlocked_sr(nor, ofs + len, nor->params->size - (ofs + len),
>   				    status_old))
>   		can_be_bottom = false;
>   
> @@ -285,7 +282,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
>   
>   	/* lock_len: length of region that should remain locked */
>   	if (use_top)
> -		lock_len = mtd->size - (ofs + len);
> +		lock_len = nor->params->size - (ofs + len);
>   	else
>   		lock_len = ofs;
>   
> ---snip---
> 
> -michael

Re: [bug] spi-nor not unlocking on Ubiquiti XW and WA

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On Mon Jun 30, 2025 at 7:50 PM CEST, Jean-Marc Ranger wrote:
> On 2025-06-30 05:25, Michael Walle wrote:
> > This seems to be due to the use of the uninitalized "mtd->size".
> > Could you try the following patch which is based on the latest
> > next kernel. It replaces mtd->size with nor->params->size, so you
> > could backport it to 6.6, but maybe it will apply anyway.
>
> Thank you so much for taking the time!
>
> Your patch applies automatically on 6.6.93, with minimal fuzzing and 
> offset changes. And it fixes the issue! Formally:
> Tested-by: Jean-Marc Ranger <jmranger@hotmail.com>

Great. I'll prepare a proper patch shortly.

But I have to wonder whether this flash is locked by default after
each power-up or if it's just locked once after it comes out of the
factory.

The MX25L12805D datasheet reads:

  BP3, BP2, BP1, BP0 bits. The Block Protect (BP3, BP2, BP1, BP0)
  bits, non-volatile bits, indicate the protected area(as defined in
  table 1) of the device to against the program/erase instruction
  without hardware protection mode being set.

Does the bootloader enable the protection bits on each bootup? If
that's the case be aware that the bits might wear out over time
depending on how often that device is restarted ;) Likely won't
happen but it's still not nice if the bootloader fights against what
linux is doing (or the other way around).

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [bug] spi-nor not unlocking on Ubiquiti XW and WA

[Jean-Marc Ranger]

On 2025-07-01 03:07, Michael Walle wrote:
> On Mon Jun 30, 2025 at 7:50 PM CEST, Jean-Marc Ranger wrote:
>> On 2025-06-30 05:25, Michael Walle wrote:
>>> This seems to be due to the use of the uninitalized "mtd->size".
>>> Could you try the following patch which is based on the latest
>>> next kernel. It replaces mtd->size with nor->params->size, so you
>>> could backport it to 6.6, but maybe it will apply anyway.
>>
>> Thank you so much for taking the time!
>>
>> Your patch applies automatically on 6.6.93, with minimal fuzzing and
>> offset changes. And it fixes the issue! Formally:
>> Tested-by: Jean-Marc Ranger <jmranger@hotmail.com>
> 
> Great. I'll prepare a proper patch shortly.
> 
> But I have to wonder whether this flash is locked by default after
> each power-up or if it's just locked once after it comes out of the
> factory.
> 
> The MX25L12805D datasheet reads:
> 
>    BP3, BP2, BP1, BP0 bits. The Block Protect (BP3, BP2, BP1, BP0)
>    bits, non-volatile bits, indicate the protected area(as defined in
>    table 1) of the device to against the program/erase instruction
>    without hardware protection mode being set.
> 
> Does the bootloader enable the protection bits on each bootup? If
> that's the case be aware that the bits might wear out over time
> depending on how often that device is restarted ;) Likely won't
> happen but it's still not nice if the bootloader fights against what
> linux is doing (or the other way around).

My understanding is that it does. What was written when the same issue 
happened in the OpenWRT 22.03 to 23.05 transition a year ago might 
interest you: [1].

Since we're on the topic of locking bits in Macronix chips, I noticed 
yesterday that OpenWRT carries a small patch from 2021 on that topic: 
[2][3]. No request here, only FYI.

Jean-Marc

[1] https://github.com/openwrt/openwrt/pull/14237
[2] 
https://github.com/openwrt/openwrt/blob/main/target/linux/generic/pending-6.12/498-mtd-spi-nor-locking-support-for-MX25L6405D.patch
[3] 
https://github.com/openwrt/openwrt/blob/main/target/linux/generic/pending-6.6/498-mtd-spi-nor-locking-support-for-MX25L6405D.patch