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-next>] [day] [month] [year] [list]
Message-ID: <09bf9abc-4a43-f1e9-d5f7-5b034c9812eb@mcqueen.au>
Date:   Wed, 11 Jan 2023 14:40:40 +1100
From:   Craig McQueen <craig@...ueen.au>
To:     netdev <netdev@...r.kernel.org>
Subject: KSZ8795 incorrect bit definitions for static MAC table

I'm working on an embedded project with KSZ8794 and kernel 5.15.

I found that after reconfiguring the network interfaces a few times, 
configurations were failing with error ENOSPC. I traced it to the 8 
slots of the static MAC table gradually filling up.

It looks as though several bit definitions for the static MAC address 
table are wrong. I made the changes below. I'm working with kernel 5.15. 
Looking at the latest git repo, it looks as though the KSZ8795 driver 
code has been reworked so ksz8795_masks[] etc is in ksz_common.c rather 
than ksz8795.c. But the bug appears to be still present, and the fix is 
similar.

Note that for KSZ8795, the static MAC address registers are unusual 
because the "FID" and "Use FID" fields have different bit locations for 
reads and writes. But also the "Override" and "Forwarding Ports" 
definitions were just wrong.

Note that the ksz8863_masks[] change has not been tested, but it looks 
right from a read of the data sheet.



commit be8988c1d3f014799a518360fe219ad35b42c9a8 (HEAD -> 
ksz8795-bit-offsets)
Author: Craig McQueen <craig.mcqueen@...errange.com 
<mailto:craig.mcqueen@...errange.com>>
Date:   Tue Jan 10 17:16:37 2023 +1100

     ksz8795: Fix bit offsets for static MAC address table

     Refer to KSZ8795CLX data sheet, Microchip document DS00002112E, section
     4.4 "Static MAC Address Table", table 4-16.

     Unusually, the bit locations of the "FID" and "Use FID" fields are
     different for reads and writes.

diff --git a/drivers/net/dsa/microchip/ksz8.h 
b/drivers/net/dsa/microchip/ksz8.h
index 9d611895d3cf..bb98cc3d89f6 100644
--- a/drivers/net/dsa/microchip/ksz8.h
+++ b/drivers/net/dsa/microchip/ksz8.h
@@ -34,8 +34,10 @@ enum ksz_masks {
      VLAN_TABLE_MEMBERSHIP,
      VLAN_TABLE_VALID,
      STATIC_MAC_TABLE_VALID,
-    STATIC_MAC_TABLE_USE_FID,
-    STATIC_MAC_TABLE_FID,
+    STATIC_MAC_TABLE_USE_FID_R,
+    STATIC_MAC_TABLE_USE_FID_W,
+    STATIC_MAC_TABLE_FID_R,
+    STATIC_MAC_TABLE_FID_W,
      STATIC_MAC_TABLE_OVERRIDE,
      STATIC_MAC_TABLE_FWD_PORTS,
      DYNAMIC_MAC_TABLE_ENTRIES_H,
@@ -51,7 +53,8 @@ enum ksz_shifts {
      VLAN_TABLE_MEMBERSHIP_S,
      VLAN_TABLE,
      STATIC_MAC_FWD_PORTS,
-    STATIC_MAC_FID,
+    STATIC_MAC_FID_R,
+    STATIC_MAC_FID_W,
      DYNAMIC_MAC_ENTRIES_H,
      DYNAMIC_MAC_ENTRIES,
      DYNAMIC_MAC_FID,
diff --git a/drivers/net/dsa/microchip/ksz8795.c 
b/drivers/net/dsa/microchip/ksz8795.c
index c9c682352ac3..16b546ad0cd3 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -50,10 +50,12 @@ static const u32 ksz8795_masks[] = {
      [VLAN_TABLE_MEMBERSHIP]        = GENMASK(11, 7),
      [VLAN_TABLE_VALID]        = BIT(12),
      [STATIC_MAC_TABLE_VALID]    = BIT(21),
-    [STATIC_MAC_TABLE_USE_FID]    = BIT(23),
-    [STATIC_MAC_TABLE_FID]        = GENMASK(30, 24),
-    [STATIC_MAC_TABLE_OVERRIDE]    = BIT(26),
-    [STATIC_MAC_TABLE_FWD_PORTS]  = GENMASK(24, 20),
+    [STATIC_MAC_TABLE_USE_FID_R]  = BIT(24),
+    [STATIC_MAC_TABLE_USE_FID_W]  = BIT(23),
+    [STATIC_MAC_TABLE_FID_R]    = GENMASK(31, 25),
+    [STATIC_MAC_TABLE_FID_W]    = GENMASK(30, 24),
+    [STATIC_MAC_TABLE_OVERRIDE]    = BIT(22),
+    [STATIC_MAC_TABLE_FWD_PORTS]  = GENMASK(20, 16),
      [DYNAMIC_MAC_TABLE_ENTRIES_H]  = GENMASK(6, 0),
      [DYNAMIC_MAC_TABLE_MAC_EMPTY]  = BIT(8),
      [DYNAMIC_MAC_TABLE_NOT_READY]  = BIT(7),
@@ -67,7 +69,8 @@ static const u8 ksz8795_shifts[] = {
      [VLAN_TABLE_MEMBERSHIP_S]    = 7,
      [VLAN_TABLE]            = 16,
      [STATIC_MAC_FWD_PORTS]        = 16,
-    [STATIC_MAC_FID]        = 24,
+    [STATIC_MAC_FID_R]        = 25,
+    [STATIC_MAC_FID_W]        = 24,
      [DYNAMIC_MAC_ENTRIES_H]        = 3,
      [DYNAMIC_MAC_ENTRIES]        = 29,
      [DYNAMIC_MAC_FID]        = 16,
@@ -100,8 +103,10 @@ static const u32 ksz8863_masks[] = {
      [VLAN_TABLE_MEMBERSHIP]        = GENMASK(18, 16),
      [VLAN_TABLE_VALID]        = BIT(19),
      [STATIC_MAC_TABLE_VALID]    = BIT(19),
-    [STATIC_MAC_TABLE_USE_FID]    = BIT(21),
-    [STATIC_MAC_TABLE_FID]        = GENMASK(29, 26),
+    [STATIC_MAC_TABLE_USE_FID_R]  = BIT(21),
+    [STATIC_MAC_TABLE_USE_FID_W]  = BIT(21),
+    [STATIC_MAC_TABLE_FID_R]    = GENMASK(29, 26),
+    [STATIC_MAC_TABLE_FID_W]    = GENMASK(29, 26),
      [STATIC_MAC_TABLE_OVERRIDE]    = BIT(20),
      [STATIC_MAC_TABLE_FWD_PORTS]  = GENMASK(18, 16),
      [DYNAMIC_MAC_TABLE_ENTRIES_H]  = GENMASK(5, 0),
@@ -116,7 +121,8 @@ static const u32 ksz8863_masks[] = {
  static u8 ksz8863_shifts[] = {
      [VLAN_TABLE_MEMBERSHIP_S]    = 16,
      [STATIC_MAC_FWD_PORTS]        = 16,
-    [STATIC_MAC_FID]        = 22,
+    [STATIC_MAC_FID_R]        = 22,
+    [STATIC_MAC_FID_W]        = 22,
      [DYNAMIC_MAC_ENTRIES_H]        = 3,
      [DYNAMIC_MAC_ENTRIES]        = 24,
      [DYNAMIC_MAC_FID]        = 16,
@@ -604,9 +610,9 @@ static int ksz8_r_sta_mac_table(struct ksz_device 
*dev, u16 addr,
          data_hi >>= 1;
          alu->is_static = true;
          alu->is_use_fid =
-            (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ? 1 : 0;
-        alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
-  shifts[STATIC_MAC_FID];
+            (data_hi & masks[STATIC_MAC_TABLE_USE_FID_R]) ? 1 : 0;
+        alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID_R]) >>
+  shifts[STATIC_MAC_FID_R];
          return 0;
      }
      return -ENXIO;
@@ -633,8 +639,8 @@ static void ksz8_w_sta_mac_table(struct ksz_device 
*dev, u16 addr,
      if (alu->is_override)
          data_hi |= masks[STATIC_MAC_TABLE_OVERRIDE];
      if (alu->is_use_fid) {
-        data_hi |= masks[STATIC_MAC_TABLE_USE_FID];
-        data_hi |= (u32)alu->fid << shifts[STATIC_MAC_FID];
+        data_hi |= masks[STATIC_MAC_TABLE_USE_FID_W];
+        data_hi |= ((u32)alu->fid << shifts[STATIC_MAC_FID_W]) & 
masks[STATIC_MAC_TABLE_FID_W];
      }
      if (alu->is_static)
          data_hi |= masks[STATIC_MAC_TABLE_VALID];


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ