[BUG] scsi: mpi3mr: logical SAS link rate is exported as physical negotiated_linkrate
From: Илья Хомяков
Date: Thu Jul 30 2026 - 15:05:28 EST
Hello.
I would like to report an incorrect SAS PHY link-rate value exported by
the mpi3mr driver through the Linux SAS transport class.
Test configuration
------------------
Controller under test:
Broadcom eHBA 9600-16e
ASIC: SAS4016 B0
PCI address: 0000:32:00.0
Driver: mpi3mr 8.17.1.0.0
Firmware: 8.17.1.0-00000-00001
Mode: Perf-IT SAS Only
Reference controller:
Broadcom 9500
ASIC: SAS3816 A0
PCI address: 0000:ca:00.0
Driver: mpt3sas 57.00.00.00
Firmware: 38.00.00.00
Both controllers are connected to the same Microchip SXP 42x24G
expander.
Observed result
---------------
For the same local expander PHYs:
PHY mpt3sas sysfs mpi3mr sysfs expander-side EOM
17 22.5 Gbit/s 1.5 Gbit/s 22.5 Gbit/s
18 12.0 Gbit/s 1.5 Gbit/s 12.0 Gbit/s
19 12.0 Gbit/s 1.5 Gbit/s 12.0 Gbit/s
24 22.5 Gbit/s 1.5 Gbit/s 22.5 Gbit/s
25 22.5 Gbit/s 1.5 Gbit/s 22.5 Gbit/s
26 22.5 Gbit/s 1.5 Gbit/s 22.5 Gbit/s
27 22.5 Gbit/s 1.5 Gbit/s 22.5 Gbit/s
28 12.0 Gbit/s 1.5 Gbit/s 12.0 Gbit/s
The value is read from:
/sys/class/sas_phy/phy-*/negotiated_linkrate
The mpt3sas value agrees with the independent physical-rate
measurement on the expander side.
Source-code analysis
--------------------
The MPI3 negotiated_link_rate byte contains:
bits 7:4 - logical negotiated rate
bits 3:0 - physical negotiated rate
mpi3mr populates sas_phy->negotiated_linkrate using:
MPI3_SAS_NEG_LINK_RATE_LOGICAL_MASK
MPI3_SAS_NEG_LINK_RATE_LOGICAL_SHIFT
mpt3sas populates the equivalent SAS transport field using the
physical low nibble:
MPI2_SAS_NEG_LINK_RATE_MASK_PHYSICAL
Linux sas_phy->negotiated_linkrate describes a physical PHY, so it
appears that mpi3mr should use the MPI3 physical low nibble in all
initial-discovery, refresh and link-update paths.
Impact
------
The incorrect value causes false 1.5 Gbit/s downshift reports in sysfs,
sasutils, monitoring and hardware-validation software, while the
physical links continue operating at 12 or 22.5 Gbit/s.
Please confirm whether this is a driver defect and whether the physical
low nibble should be used for sas_phy->negotiated_linkrate.
Example with two 9500 and 9600 controllers connected to one expander:
sg_map -x -i
/dev/sg0 8 0 0 0 0 /dev/sda ATA Samsung SSD 860 4B6Q
/dev/sg1 16 0 0 0 13 Adaptec Smart Adapter 8.23
/dev/sg2 16 2 0 0 12 Adaptec 1100-8e 8.23
/dev/sg3 15 0 0 0 0 /dev/sdb SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg4 15 0 1 0 0 /dev/sdc HGST HUH721212AL4204 C3F0
/dev/sg5 15 0 2 0 0 /dev/sdd HGST HUH721212AL4204 C3F0
/dev/sg6 15 0 3 0 0 /dev/sde SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg7 15 0 4 0 0 /dev/sdf SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg8 15 0 5 0 0 /dev/sdg SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg9 15 0 6 0 0 /dev/sdh SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg10 15 0 7 0 0 /dev/sdi HGST HUH721212AL4204 C3D0
/dev/sg11 15 0 8 0 13 SXP 42x24G RevB (from 9500)
/dev/sg12 0 0 0 0 0 /dev/sdj SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg13 0 0 1 0 0 /dev/sdk HGST HUH721212AL4204 C3F0
/dev/sg14 0 0 2 0 0 /dev/sdl HGST HUH721212AL4204 C3F0
/dev/sg15 0 0 3 0 0 /dev/sdm SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg16 0 0 4 0 0 /dev/sdn SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg17 0 0 5 0 0 /dev/sdo SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg18 0 0 6 0 0 /dev/sdp SAMSUNG MZILG3T8HCLS/A07 GXG3
/dev/sg19 0 0 7 0 0 /dev/sdq HGST HUH721212AL4204 C3D0
/dev/sg20 0 0 8 0 13 SXP 42x24G RevB (from 9600)
>From 9600-16e:
./microchip_page84_probe_v3.py phys --device /dev/sg20
SXP PM8xxx local PHYs for /dev/sg20
PHY Link Negotiated Invalid Disp LossSync
Reset
------------------------------------------------------------------------------------------
0 UP 22.5 Gbit 0 0 3
0
1 UP 22.5 Gbit 0 0 3
0
2 UP 22.5 Gbit 0 0 3
0
3 UP 22.5 Gbit 0 0 3
0
4 DOWN Unknown 0 0 0 0
5 DOWN Unknown 0 0 0 0
6 DOWN Unknown 0 0 0 0
7 DOWN Unknown 0 0 0 0
8 DOWN Unknown 0 0 0 0
9 DOWN Unknown 0 0 0 0
10 DOWN Unknown 0 0 0 0
11 DOWN Unknown 0 0 0 0
12 UP 12.0 Gbit 827 767 4 0
13 UP 12.0 Gbit 22 22 4 0
14 UP 12.0 Gbit 1930 1883 5 0
15 UP 12.0 Gbit 386 386 5 0
16 DOWN Unknown 0 0 0 0
17 UP 1.5 Gbit 0 0 0
0
18 UP 1.5 Gbit 0 0 0
0
19 UP 1.5 Gbit 0 0 0
0
20 DOWN Unknown 0 0 0 0
21 DOWN Unknown 0 0 0 0
22 DOWN Unknown 0 0 0 0
23 DOWN Unknown 0 0 0 0
24 UP 1.5 Gbit 325 324 1 0
25 UP 1.5 Gbit 0 0 0
0
26 UP 1.5 Gbit 0 0 0
0
27 UP 1.5 Gbit 0 0 0
0
28 UP 1.5 Gbit 0 0 0
0
29 DOWN Unknown 0 0 0 0
30 DOWN Unknown 0 0 0 0
31 DOWN Unknown 0 0 0 0
32 DOWN Phy disabled 0 0 0 0
33 DOWN Phy disabled 0 0 0 0
34 DOWN Phy disabled 0 0 0 0
35 DOWN Phy disabled 0 0 0 0
36 DOWN Phy disabled 0 0 0 0
37 DOWN Phy disabled 0 0 0 0
38 DOWN Phy disabled 0 0 0 0
39 DOWN Phy disabled 0 0 0 0
40 DOWN Phy disabled 0 0 0 0
41 DOWN Phy disabled 0 0 0 0
.
>From 9500-16e
./microchip_page84_probe_v3.py phys --device /dev/sg11
SXP PM8xxx local PHYs for /dev/sg11
PHY Link Negotiated Invalid Disp LossSync
Reset
------------------------------------------------------------------------------------------
0 UP 22.5 Gbit 0 0 3
0
1 UP 22.5 Gbit 0 0 3
0
2 UP 22.5 Gbit 0 0 3
0
3 UP 22.5 Gbit 0 0 3
0
4 DOWN Unknown 0 0 0 0
5 DOWN Unknown 0 0 0 0
6 DOWN Unknown 0 0 0 0
7 DOWN Unknown 0 0 0 0
8 DOWN Unknown 0 0 0 0
9 DOWN Unknown 0 0 0 0
10 DOWN Unknown 0 0 0 0
11 DOWN Unknown 0 0 0 0
12 UP 12.0 Gbit 827 767 4 0
13 UP 12.0 Gbit 22 22 4 0
14 UP 12.0 Gbit 1930 1883 5 0
15 UP 12.0 Gbit 386 386 5 0
16 DOWN Unknown 0 0 0 0
17 UP 22.5 Gbit 0 0 0
0
18 UP 12.0 Gbit 0 0 0
0
19 UP 12.0 Gbit 0 0 0
0
20 DOWN Unknown 0 0 0 0
21 DOWN Unknown 0 0 0 0
22 DOWN Unknown 0 0 0 0
23 DOWN Unknown 0 0 0 0
24 UP 12.0 Gbit 325 324 1 0
25 UP 12.0 Gbit 0 0 0
0
26 UP 12.0 Gbit 0 0 0
0
27 UP 12.0 Gbit 0 0 0
0
28 UP 12.0 Gbit 0 0 0
0
29 DOWN Unknown 0 0 0 0
30 DOWN Unknown 0 0 0 0
31 DOWN Unknown 0 0 0 0
32 DOWN Phy disabled 0 0 0 0
33 DOWN Phy disabled 0 0 0 0
34 DOWN Phy disabled 0 0 0 0
35 DOWN Phy disabled 0 0 0 0
36 DOWN Phy disabled 0 0 0 0
37 DOWN Phy disabled 0 0 0 0
38 DOWN Phy disabled 0 0 0 0
39 DOWN Phy disabled 0 0 0 0
40 DOWN Phy disabled 0 0 0 0
41 DOWN Phy disabled 0 0 0 0
Regards,
Ilya Khomyakov
khomyakovilya@xxxxxxxxx
Attachment:
0001-scsi-mpi3mr-report-the-physical-SAS-PHY-link-rate.patch
Description: Binary data
## 1. Summary
Two independent defects in the SAS Transport Layer have been confirmed on a Broadcom eHBA 9600 / SAS4016 B0 running the `mpi3mr 8.17.1.0.0` driver:
1. **Incorrect population of `sas_phy->negotiated_linkrate`.** The driver takes the **logical negotiated link rate** from the high nibble of the `negotiated_link_rate` byte, whereas the Linux SAS Transport Layer expects the physical PHY rate from the low nibble. As a result, Linux sysfs reports `1.5 Gbit` for physical links that are actually running at `12 Gbit/s` and `22.5 Gbit/s`.
2. **Genuine undefined behavior when building `phy_mask`.** The mask field is declared `u64`, but its bits are set using expressions of the form `(1 << i)`, where `1` has type 32-bit signed `int`. For `i >= 31` this expression is undefined behavior. On an expander with 46 PHYs this reproduces under UBSAN at `mpi3mr_transport.c:1095`. In addition, `ffs()` is applied to a `u64` even though it operates on `int`, so the upper 32 bits of the mask are lost.
Separately, UBSAN reports `array-index-out-of-bounds` on accesses to `phy_data[i]`. In the shipped MPI3 header the field is declared as a single-element trailing array `phy_data[1]`, while the driver dynamically allocates a larger buffer. This is most likely a violation of the declared array bound / an obsolete trailing-array idiom rather than a proven overrun of the actually allocated heap buffer. Even so, this code should be converted to a flexible-array member and supplemented with a check of the actual page length.
### Confidence levels
| Finding | Confidence | Basis |
| `mpi3mr` substitutes the logical rate for the physical rate| Very high | Bit-level MPI3 definition, exact source lines, comparison against `mpt3sas`, independent measurement on the Microchip expander |
| `(1 << i)` causes UB | Certain | C language rules, exact source, actual UBSAN report |
| `ffs(u64)` loses the upper bits | Certain | `phy_mask` is `u64`, the `ffs()` interface takes `int` |
| The UBSAN array-index report means real memory corruption | Not proven | The buffer is allocated dynamically based on `num_phys`, but the struct type declares an array of length 1 |
### Upstream status as of 30 July 2026
A review of the current Linux `master` tree shows that the defects are not limited to the `8.17.1.0.0` vendor package:
- `drivers/scsi/mpi3mr/mpi3mr_transport.c` still populates `sas_phy->negotiated_linkrate` via `MPI3_SAS_NEG_LINK_RATE_LOGICAL_MASK` for both host PHYs and expander PHYs;
- `struct mpi3mr_sas_port` still contains `u64 phy_mask`, yet the add/delete paths use `(1 << phy_id)` followed by `ffs(phy_mask)`;
- by contrast, the current `mpt3sas_transport.c` still uses `MPI2_SAS_NEG_LINK_RATE_MASK_PHYSICAL` when populating the same Linux SAS Transport Layer field.
The issue should therefore be raised with Broadcom and with upstream `linux-scsi` at the same time: a fix is needed for the current kernel, plus a backport into the distributed Broadcom driver package.
## 2. Configuration under test
### Controller 9600
- Broadcom eHBA 9600-16e, ASIC `SAS4016 B0`.
- PCI address: `0000:32:00.0`.
- WWID: `500062b2:20ee9500`.
- Firmware: `8.17.1.0-00000-00001`, firmware package `8.17.1.0.00000-00002`.
- Mode during reproduction: `Perf-IT (SAS Only)`.
- Linux driver: `mpi3mr 8.17.1.0.0`.
- The driver reports 39 controller PHYs and a 46-PHY expander.
### Reference controller 9500
- Broadcom HBA 9500 / ASIC `SAS3816 A0`.
- PCI address: `0000:ca:00.0`.
- WWID: `500062b2:09e05740`.
- Firmware: `38.00.00.00`.
- Linux driver: `mpt3sas 57.00.00.00`.
### Shared topology
Both controllers are attached to the same Microchip SXP 24G, presented as `SXP 42x24G RevB`. The same expander is reachable:
- through `mpt3sas`: `/dev/sg11`;
- through `mpi3mr`: `/dev/sg20`.
This is precisely why the comparison does not depend on different drives, different cables or different expanders: the values are read for the *same* local Microchip PHYs through two independent initiator paths.
## 3. Defect #1: the logical rate is used as the physical PHY rate
### 3.1. MPI3 field format
`mpi/mpi30_cnfg.h` of the `mpi3mr 8.17.1.0.0` driver defines:
```c
#define MPI3_SAS_NEG_LINK_RATE_LOGICAL_MASK (0xf0)
#define MPI3_SAS_NEG_LINK_RATE_LOGICAL_SHIFT (4)
#define MPI3_SAS_NEG_LINK_RATE_PHYSICAL_MASK (0x0f)
#define MPI3_SAS_NEG_LINK_RATE_PHYSICAL_SHIFT (0)
```
A single byte therefore carries **two independent values**:
- bits `[7:4]` — logical negotiated link rate;
- bits `[3:0]` — physical negotiated link rate.
The Linux `sas_phy` object and the `negotiated_linkrate` sysfs attribute are meant to describe the state of a specific physical PHY. This is also borne out by the long-established behaviour of `mpt3sas`, which selects the low physical nibble for the same attribute.
### 3.2. What `mpi3mr` does
In the shipped `mpi3mr_transport.c`, the logical high nibble is used in every principal path that sets or refreshes the rate:
| Path | Vendor source lines | Actual operation |
|---|---:|---|
| Host PHY add | 1348–1351 | `LOGICAL_MASK >> LOGICAL_SHIFT` |
| Expander PHY add | 1419–1422 | `LOGICAL_MASK >> LOGICAL_SHIFT` |
| Rate refresh after PHY operation | 2195–2199 | `LOGICAL_MASK >> LOGICAL_SHIFT` |
| Expander link update | 2833–2835 | `LOGICAL_MASK >> LOGICAL_SHIFT` |
| Host PHY refresh | 3083 | direct `>> 4` |
| Getting the negotiated rate for a target | 3324–3326 | `LOGICAL_MASK >> LOGICAL_SHIFT` |
Example from `mpi3mr_add_expander_phy()`:
```c
phy->negotiated_linkrate = mpi3mr_convert_phy_link_rate(
(expander_pg1.negotiated_link_rate &
MPI3_SAS_NEG_LINK_RATE_LOGICAL_MASK) >>
MPI3_SAS_NEG_LINK_RATE_LOGICAL_SHIFT);
```
That is, the code deliberately extracts the high nibble even though a separate `PHYSICAL_MASK` is defined right next to it.
### 3.3. What `mpt3sas` does
In `mpt3sas_transport.c`, the physical low nibble is used for the same Linux SAS Transport Layer field:
```c
phy->negotiated_linkrate = _transport_convert_phy_link_rate(
phy_pg0.NegotiatedLinkRate &
MPI2_SAS_NEG_LINK_RATE_MASK_PHYSICAL);
```
The same is done for expander PHYs and for the PHY Page 0 re-read. Principal vendor source locations:
- host PHY: lines 1179–1180;
- expander PHY: lines 1258–1260;
- refresh: lines 2125–2127.
### 3.4. Hardware comparison of `mpi3mr` and `mpt3sas`
Values below are for the *same* Microchip local PHYs. The Microchip column gives an independent classification of the physical rate obtained from page `0x84` EOM capture.
| Microchip PHY | Via 9500 / `mpt3sas` sysfs | Via 9600 / `mpi3mr` sysfs | Microchip physical-rate evidence | Verdict |
| 17 | 22.5 Gbit/s | 1.5 Gbit/s | 22.5 Gbit/s | `mpi3mr` wrong |
| 18 | 12.0 Gbit/s | 1.5 Gbit/s | 12.0 Gbit/s | `mpi3mr` wrong |
| 19 | 12.0 Gbit/s | 1.5 Gbit/s | 12.0 Gbit/s | `mpi3mr` wrong |
| 24 | 22.5 Gbit/s | 1.5 Gbit/s | 22.5 Gbit/s | `mpi3mr` wrong |
| 25 | 22.5 Gbit/s | 1.5 Gbit/s | 22.5 Gbit/s | `mpi3mr` wrong |
| 26 | 22.5 Gbit/s | 1.5 Gbit/s | 22.5 Gbit/s | `mpi3mr` wrong |
| 27 | 22.5 Gbit/s | 1.5 Gbit/s | 22.5 Gbit/s | `mpi3mr` wrong |
| 28 | 12.0 Gbit/s | 1.5 Gbit/s | 12.0 Gbit/s | `mpi3mr` wrong |
Control PHYs 0–3 and 12–15 report identical values through both drivers, because on those PHYs the logical and physical nibbles happen to match in this state. The defect can therefore stay unnoticed for a long time in a homogeneous topology and only shows up on PHYs where the two fields differ.
### 3.5. Why this is not a Microchip tooling error
The observation is corroborated by three independent lines of evidence:
1. `mpt3sas`, on the same expander and the same PHYs, reports 12/22.5 Gbit/s.
2. Microchip EOM capture yields physical count rates consistent with 12/22.5 Gbit/s.
3. `mpi3mr` is the only source of the 1.5 Gbit/s value, and its source code explicitly selects the logical high nibble.
For a conclusive vendor capture it would also help to record the raw `negotiated_link_rate` byte from MPI3 SAS PHY Page 0 / SAS Expander Page 1. Based on the observed result, the expected combinations are a logical code equivalent to `1.5G` in the high nibble and a physical code of `12G`/`22.5G` in the low nibble; this should be confirmed from the raw byte rather than treated as already proven.
### 3.6. Impact
The defect probably does not change the actual data-path rate: it misreports the state to the Linux SAS Transport Layer. The operational consequences are nevertheless significant:
- `sysfs` shows a spurious downshift to 1.5 Gbit/s;
- `sasutils`, inventory, monitoring and in-house diagnostic scripts receive incorrect data;
- automation may trigger unnecessary resets, cable/drive replacements, or wrongly condemn a link;
- errors and eye measurements cannot be reliably correlated with the PHY rate;
- topology refresh and link-up events keep overwriting the value with the same incorrect logical nibble.
### 3.7. Required direction of the fix
Broadcom should confirm the semantics of the MPI3 field and then consistently change **every path that populates the Linux `sas_phy->negotiated_linkrate` or passes a link rate into `mpi3mr_update_links()`** so that it uses:
- `MPI3_SAS_NEG_LINK_RATE_PHYSICAL_MASK`;
- `MPI3_SAS_NEG_LINK_RATE_PHYSICAL_SHIFT`.
The logical rate may be retained for internal firmware/connection-management purposes, but it must not be published as the physical `sas_phy` rate. A point fix in `mpi3mr_add_expander_phy()` alone is not sufficient: the error would return on the next refresh/link event.
## 4. Defect #2: 32-bit shift against a 64-bit `phy_mask`
### 4.1. Type mismatch
In `mpi3mr.h`:
```c
u64 phy_mask;
```
But `mpi3mr_transport.c` contains expressions such as:
```c
mr_sas_port->phy_mask |= (1 << i);
mr_sas_port->phy_mask &= ~(1 << mr_sas_phy->phy_id);
h_port[j].phy_mask |= (1 << i);
h_port[port_idx].phy_mask = (1 << i);
```
The left operand `1` has type signed `int`. The shift is evaluated **before** the assignment into the `u64`, so the 64-bit width of the field does not make the expression safe.
### 4.2. Why this is undefined behavior
For a conventional 32-bit `int`:
- `1 << 31` attempts to set the sign bit and has no correctly representable positive result;
- `1 << 32` and any wider shift have a shift count not smaller than the width of the type.
Both cases are undefined behavior in C. On a 46-PHY expander, local PHY identifiers 32–45 are valid, so the problem is not theoretical.
### 4.3. Actual reproduction
From the runtime log:
```text
UBSAN: shift-out-of-bounds in mpi3mr_transport.c:1095:31
Workqueue: mpi3mr0_fwevt_wrkr
mpi3mr_sas_port_add ...
mpi3mr_update_links ...
mpi3mr_report_tgtdev_to_sas_transport ...
```
Source context for line 1095:
```c
for (i = 0; i < mr_sas_node->num_phys; i++) {
...
mr_sas_port->phy_mask |= (1 << i);
}
```
Immediately before this, the driver reports:
```text
host_add: ... phys(39)
expander_add: ... phys(46)
```
After eight drives have been added, the warning appears just before the expander/SES target is added. In the parallel `mpt3sas` topology log that target is bound to expander PHY 43, which is consistent with a `1 << 43` trigger.
### 4.4. A second error: `ffs()` on a `u64`
After the mask has been built, the code performs:
```c
mr_sas_port->lowest_phy = ffs(mr_sas_port->phy_mask) - 1;
```
`ffs()` operates on `int`. Passing a `u64` truncates it to the low 32 bits. Consequences:
- if only a PHY >= 32 is set, `ffs()` sees zero and `lowest_phy` becomes `-1`;
- if both low and high bits are set simultaneously, the upper half is ignored entirely;
- the logic for wide-port membership, port removal and lowest-PHY selection becomes unreliable.
### 4.5. Potential consequences
- loss of PHY 31–63 membership in the mask;
- incorrect `lowest_phy`;
- incorrect wide-port aggregation;
- incorrect PHY add/remove;
- instability in the topology refresh, hotplug and reset paths;
- possible arbitrary behavioural changes depending on compiler/optimization level.
The observed incorrect link rate of 1.5 Gbit/s **should not automatically be attributed to this UB**. The rate error has a separate, direct cause — the choice of the logical nibble. The two defects must be fixed and tested independently.
### 4.6. Comparison with `mpt3sas`
The `mpt3sas` vendor source also contains legacy `(1 << i)` expressions, but they are applied to the HBA port mask only when `sas_node` is the controller host node. In this configuration the 9500 reports 21 host PHYs, so a shift >= 31 is never reached. For the 46-PHY expander, `mpt3sas` does not build this host mask and no UBSAN warning appears.
This does **not** prove that all versions of `mpt3sas` are universally free of similar 32-bit shifts. It shows that:
- this particular `mpt3sas 57.00.00.00`, in the topology under test, does not trigger UBSAN;
- `mpi3mr` stores the per-port mask as a `u64` but, in vendor code, builds it with a 32-bit expression and genuinely passes through PHY 43.
### 4.7. Required direction of the fix
A full audit of every `phy_mask` operation is needed:
- build bits with a 64-bit operation (`BIT_ULL()` or equivalent);
- validate `phy_id < 64`; if a future ASIC/expander may have more than 64 PHYs, use an appropriately sized bitmap;
- replace `ffs()` with a 64-bit find-first-bit or a bitmap API;
- handle the zero-mask case explicitly, rather than accidentally computing `-1`;
- review the vendor occurrences near lines 882, 909, 1095, 2518 and 2545.
## 5. UBSAN `array-index-out-of-bounds` on `phy_data[i]`
### 5.1. Observed messages
UBSAN warnings were recorded during initialization/refresh:
- `mpi3mr_transport.c:3180:32`;
- `mpi3mr_transport.c:3200:38`;
- `mpi3mr_transport.c:3078:32`;
- `mpi3mr_transport.c:3083:32`;
- `mpi3mr_transport.c:3087:38`;
- `mpi3mr_transport.c:3105:21`.
All of them relate to accesses of the form `sas_io_unit_pg0->phy_data[i]`.
### 5.2. The warning stems from the struct type
In the vendor `mpi30_cnfg.h`:
```c
#define MPI3_SAS_IO_UNIT0_PHY_MAX (1)
...
struct mpi3_sas_io_unit_page0 {
...
struct mpi3_sas_io_unit0_phy_data phy_data[MPI3_SAS_IO_UNIT0_PHY_MAX];
};
```
The driver then allocates:
```c
offsetof(struct mpi3_sas_io_unit_page0, phy_data) +
num_phys * sizeof(struct mpi3_sas_io_unit0_phy_data)
```
So the actual allocation is sized for `num_phys`, while the static type still tells the compiler that only index 0 is valid. UBSAN therefore legitimately flags a violation of the declared array bound.
### 5.3. What is proven and what is not
Proven:
- the vendor source uses an obsolete one-element tail;
- accesses to `phy_data[i]` with `i > 0` violate the bound known to the compiler;
- the UBSAN warnings are reproducible.
Not proven from these messages alone:
- that a read/write went beyond the actually allocated `kzalloc()` buffer;
- that the firmware returned a page shorter than the requested size.
This should therefore be described to Broadcom as a **type/bounds UB and sanitizer-visible defect**, and not, without further data, as confirmed heap corruption.
### 5.4. Recommended fix
- replace the one-element tail with a flexible-array member;
- compute the allocation with `struct_size()`;
- validate the page length and `num_phys` from the Config reply before iterating;
- bound the loop by the minimum of the actual descriptor count, the page length and the allocation capacity.
## 6. Driver comparison summary
| Aspect | `mpi3mr 8.17.1.0.0` / SAS4016 | `mpt3sas 57.00.00.00` / SAS3816 |
| Source for `sas_phy->negotiated_linkrate` | Logical high nibble | Physical low nibble |
| Result on PHYs 17–19, 24–28 | Spurious 1.5 Gbit/s | Matches Microchip 12/22.5 Gbit/s |
| Behaviour on PHYs where the nibbles match | Error hidden | Correct |
| Port mask type | `u64` | HBA mask in legacy topology structures |
| Bit construction | `(1 << phy_id)` | Legacy shift also present, but limited in this test to a host node with 21 PHYs |
| UBSAN shift warning | Reproduces, line 1095 | Not observed in the provided log |
| 46-PHY expander | Mask built in the problematic vendor `mpi3mr` path | Expander index is not added to the host HBA mask in the path under test |
| UBSAN array-bound warnings | Reproduce, due to `phy_data[1]` | Absent from the provided log |
## 7. Minimal reproducer for Broadcom
1. Attach a SAS4016/9600 and a SAS3816/9500 to the same SAS-4 expander with mixed-rate target PHYs.
2. On the 9600, select PerfIT SAS Only so that `mpi3mr` creates the SAS Transport Layer. Do not use FeatureHBA for the primary reproducer: with firmware-managed multipath, SAS transport objects may be absent by design.
3. Boot a kernel built with `CONFIG_UBSAN=y` and the bounds/shift checks enabled.
4. Capture:
- `uname -a`;
- `modinfo mpi3mr` and `modinfo mpt3sas`;
- firmware/package versions of both HBAs;
- `lspci -nnvv`;
- `dmesg` from probe onwards;
- every `/sys/class/sas_phy/*/negotiated_linkrate` along both initiator paths;
- SMP DISCOVER or expander vendor status showing the physical negotiated rate;
- the raw MPI3 SAS PHY Page 0 / SAS Expander Page 1 `negotiated_link_rate` byte.
5. Compare the same local expander PHY through `mpt3sas` and through `mpi3mr`.
6. For the UB, make sure a target/SEP is present on an expander PHY identifier >= 32, for example PHY 43.
### Expected result
- sysfs `negotiated_linkrate` matches the physical low nibble, SMP DISCOVER and the expander physical PHY rate;
- PHY identifiers 0–63 are represented safely in the `u64` mask;
- UBSAN reports no shift or array-bound violations.
### Actual result
- `mpi3mr` publishes the logical high nibble and shows 1.5 Gbit/s instead of 12/22.5 Gbit/s;
- UBSAN reports `shift-out-of-bounds` in `mpi3mr_sas_port_add()`;
- UBSAN reports `array-index-out-of-bounds` when reading `phy_data[i]`.
## 8. Proposed regression tests
### Link-rate tests
- Physical/logical pairs: `1.5/1.5`, `1.5/12`, `1.5/22.5`, `12/12`, `22.5/22.5`.
- Host PHYs and expander PHYs.
- Initial add, device add, topology refresh, link-up event, link reset, expander reset and hotplug.
- Verify that a repeated event does not restore the logical rate in sysfs.
### Mask tests
- PHY identifiers: 0, 30, 31, 32, 43, 45, 63.
- Add/remove of a single PHY and of a wide port with several PHYs on both sides of the 32-bit boundary.
- Zero mask after the last PHY is removed.
- Verification of `lowest_phy` and of full membership.
### Page-bound tests
- `num_phys = 1`, 21, 39, 46 and 64.
- A truncated Config reply and an inconsistency between page length and `num_phys`.
- UBSAN/KASAN/KMSAN builds.
## 9. Conclusion
The rate error is a direct, reproducible semantic bug: `mpi3mr` exports the logical rate where `mpt3sas` and the actual PHY use the physical rate. The undefined behavior around `phy_mask` is a separate, confirmed sanitizer defect and is particularly dangerous on a 39/46-PHY topology. The UBSAN array-bound warnings call for a backported flexible-array conversion and strict Config Page size validation, but on their own they do not yet prove a real overrun of the allocation.