[v2,1/1] ARM: orion5x: use mac_pton() helper

Stefan Hellermann stefan at the2masters.de
Thu Feb 22 15:18:11 PST 2018


Am 22.02.2018 um 22:42 schrieb Andrew Lunn:
> On Thu, Feb 22, 2018 at 06:45:51PM +0100, Stefan Hellermann wrote:
>> Hi!
>>
>> My QNAP TS-209 NAS Device is crashing with the following commit, which went
>> in the kernel as commit 4904dbda41c860fd117b20f3c48adb2780eee37e
>>
>> I cannot provide a boot log, the device panics before enabling the serial
>> console.
> Hi Stefan
>
> Did you try earlyprintk? You might need to recompile your kernel to
> enable it.
Yes, I tried it, but it didn't help. I even changed qnap_ts209_init() to 
configure uart before ethernet, but it didn't help either.
>
> Looking at the code, i don't see anything obviously wrong. So i think
> i would start by looking how many times it goes through the loop in
> qnap_tsx09_find_mac_addr() with this patch reverted, and what address
> it finds the MAC address at.
>
> Then see what happens with the current crashing code. Is it failing to
> recognise the MAC address, and so keep looping around?
I think I found the failure:
The code is looping through an ext2 filesystem on a 384kB mtd partition 
(factory configuration put there by QNAP). There it looks on every 1kB 
boundary if there is a valid MAC address. The filesystem has a 1kB block 
size, so this seems to work. The MAC address is on the 37th 1kB block.

But: On the 27th block is a large file (1,5kB) without 0 bytes inside.
The code in qnap_tsx09_find_mac_addr() maps 1kB into memory (not a whole 
file or the whole 384kB) and then calls qnap_tsx09_check_mac_addr() -> 
mac_pton() -> strlen() on this memory block. as there is no 0 byte in 
the file on the 27th block, strlen runs into bad memory and the machine 
panics. The old code had no strlen().

I changed mac_pton() to use strnlen(), and now the panic is gone. I 
don't know why strlen is actually needed in mac_pton. The string is 
checked in the following loop, if there is a zero byte somewhere, the 
loop will be returned immediately. So I think the strlen() superfluous. 
Is the following patch correct?

diff --git a/lib/net_utils.c b/lib/net_utils.c
index 148fc6e99ef6..e7785cf20f59 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -7,10 +7,6 @@ bool mac_pton(const char *s, u8 *mac)
  {
      int i;

-    /* XX:XX:XX:XX:XX:XX */
-    if (strlen(s) < 3 * ETH_ALEN - 1)
-        return false;
-
      /* Don't dirty result unless string is valid MAC. */
      for (i = 0; i < ETH_ALEN; i++) {
          if (!isxdigit(s[i * 3]) || !isxdigit(s[i * 3 + 1]))




More information about the linux-arm-kernel mailing list