[BUG][UBSAN] scsi: mpi3mr: 32-bit shift and ffs() are used with a u64 SAS PHY mask

[Илья Хомяков]

Hello,

I would like to report a reproducible undefined shift in the mpi3mr SAS
transport code when an expander has PHY identifiers greater than 31.

Test configuration
------------------

  Broadcom eHBA 9600-16e
  ASIC: SAS4016 B0
  Driver: mpi3mr 8.17.1.0.0
  Firmware: 8.17.1.0-00000-00001
  Mode: Perf-IT SAS Only

The attached Microchip expander has 46 PHYs.

Observed UBSAN report
---------------------

The driver reports:

  host_add: ... phys(39)
  expander_add: ... phys(46)

During topology discovery UBSAN reports:

  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

Relevant source pattern
-----------------------

struct mpi3mr_sas_port contains:

  u64 phy_mask;

but the mask is modified using expressions equivalent to:

  phy_mask |= 1 << phy_id;
  phy_mask &= ~(1 << phy_id);

The literal 1 has type signed int, so the shift is performed in a
32-bit signed type before assignment to u64. PHY identifiers greater
than or equal to 31 can therefore invoke undefined behavior.

The code also uses:

  lowest_phy = ffs(phy_mask) - 1;

ffs() takes an int and therefore discards the upper 32 bits of the u64
mask. A port containing only PHY identifiers above 31 can produce an
incorrect lowest_phy value.

Expected correction
-------------------

The mask operation should use a 64-bit value, for example BIT_ULL(),
with a bounds check for the PHY identifier.

The lowest set bit should be obtained using a 64-bit operation such as
__ffs64(), with explicit handling of an empty mask.

Impact
------

This can affect:

  - PHY membership in a SAS wide port;
  - add/remove handling for PHY identifiers above 31;
  - lowest-PHY selection;
  - topology refresh and hotplug;
  - behavior after controller or link reset.

Please confirm whether this is a known issue. I can test a proposed
patch on the same 46-PHY expander.

Example.
Driver messages during boot (UBSAN text)
[   23.287438] Loading mpi3mr version 8.17.1.0.0
[   23.287877] mpi3mr 0000:32:00.0: PCI device is: mpi3mr0
[   23.287885] mpi3mr0: host protection capabilities enabled  DIF1 DIF2 DIF3
[   23.288281] mpi3mr0: iomem(0x000022ffffe00000),
mapped(0x000000009882f41c), size(16384)
[   23.288289] mpi3mr0: number of MSI-X vectors found in capabilities:128
[   23.288295] mpi3mr0: ioc_status(0x00000001),
ioc_config(0x00470001), ioc_info(0x00000000ff000000) at the bringup
[   23.288300] mpi3mr0: ready timeout: 510 seconds
[   23.288304] mpi3mr0: controller is in ready state during detection
[   23.288308] mpi3mr0: issuing message unit reset (MUR) to bring to reset state
[   23.288311] mpi3mr0: issuing message unit reset(MUR)
[   23.396057] mpi3mr0: ioc_status/ioc_config after successful message
unit reset is (0x00000010)/(0x00470000)
[   23.396102] mpi3mr0: bringing controller to ready state
[   24.436128] mpi3mr0: successfully transitioned to ready state
[   24.436687] mpi3mr0: ioc_num(0), max_op_req_queues (127),
max_op_reply_queues(127), max_requests(8192), max_msix_vectors(128)
[   24.436696] mpi3mr0: max_device_handles(1374),
min_device_handles(1), max_perst_ids(2048)
[   24.436700] mpi3mr0: sge_modifier_mask(0x80),
sge_modifier_value(0x80), sge_modifier_shift(0x18)
[   24.436704] mpi3mr0: dma_mask(63),
initial_port_enable_status(0x20), max_data_len (8388608)
[   24.436709] mpi3mr0: diag_trace_size(32KB), diag_fw_size(256KB),
diag_drvr_size(0KB)
[   24.436714] mpi3mr0: max_dev_per_throttle_group(0),
max_throttle_groups(0), io_throttle_data_len(0KiB),
io_throttle_high(0MiB), io_throttle_low(0MiB)
[   24.436728] mpi3mr0: changing DMA mask from 0xFFFFFFFFFFFFFFFF to
0x7FFFFFFFFFFFFFFF
[   24.436797] mpi3mr0: running in Enhanced HBA Personality
[   24.436801] mpi3mr0: firmware version(8.17.1.0-00000-00001)
[   24.436809] mpi3mr0: Protocol=(Initiator), Capabilities=()
[   24.477142] mpi3mr0: reply buf pool(0x000000003884b1f3):
depth(8224), frame_size(128), pool_size(1028 kB),
reply_dma(0xff000000)
[   24.477150] mpi3mr0: reply_free_q pool(0x00000000525cd9ca):
depth(8225), frame_size(8), pool_size(64 kB), reply_dma(0xfefe0000)
[   24.477153] mpi3mr0: sense_buf pool(0x000000003b3652c5):
depth(2730), frame_size(256), pool_size(682 kB), sense_dma(0xfef00000)
[   24.477156] mpi3mr0: sense_buf_q pool(0x000000009f325940):
depth(2731), frame_size(8), pool_size(21 kB), sense_dma(0xfedf8000)
[   24.479434] mpi3mr0: firmware package version(8.17.1.0.00000-00002)
[   24.479552] mpi3mr0: MSI-X vectors supported: 128, no of cores: 112,
[   24.479553] mpi3mr0: MSI-x vectors requested: 113 poll_queues 0
[   24.490668] mpi3mr0: trying to create 112 operational queue pairs
[   24.490678] mpi3mr0: allocating operational queues through segmented queues
[   24.545288] mpi3mr0: successfully created 112 operational queue
pairs(default/polled) queue = (112/0)
[   24.545888] mpi3mr0: driver diag buffer level = minidump.
[   24.546126] mpi3mr0: driver diag buffer of size 516KB posted successfully
[   24.546131] mpi3mr0: controller initialization completed successfully
[   24.549355] mpi3mr0: scan started, issuing port enable
[   24.554160] mpi3mr0: hba_port entry: 0000000055e172b5, port: 0 is
added to hba_port list
[   24.554757] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3180:32
[   24.554787] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.554868]  mpi3mr_sas_host_add+0x88b/0x8c0 [mpi3mr]
[   24.554908]  mpi3mr_fwevt_worker+0xd68/0x1440 [mpi3mr]
[   24.555305] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3200:38
[   24.555326] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.555371]  mpi3mr_sas_host_add+0x89f/0x8c0 [mpi3mr]
[   24.555397]  mpi3mr_fwevt_worker+0xd68/0x1440 [mpi3mr]
[   24.559698] mpi3mr0: hba_port entry: 000000001c5af7ef, port: 8 is
added to hba_port list
[   24.561124] mpi3mr0: port enable is successfully completed
[   24.563737] mpi3mr0: hba_port entry: 000000001d363a53, port: 255 is
added to hba_port list
[   24.569591] mpi3mr0: host_add: handle(0x053f),
sas_addr(0x500062b220ee9500), phys(39)
[   24.570316] mpi3mr0: expander_add: handle(0x0001), parent(0x053f),
sas_addr(0x500e004aabcd337f), phys(46)
[   24.571125]  expander-0:0: mpi3mr_sas_port_add: added:
handle(0x0001), sas_address(0x500e004aabcd337f)
[   24.602087] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3078:32
[   24.602118] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.602179]  mpi3mr_sas_host_refresh+0x35a/0x3e0 [mpi3mr]
[   24.602206]  mpi3mr_report_tgtdev_to_sas_transport+0x69/0x3e0 [mpi3mr]
[   24.602228]  mpi3mr_report_tgtdev_to_host.isra.0+0x1dc/0x250 [mpi3mr]
[   24.602246]  mpi3mr_fwevt_worker+0x7b2/0x1440 [mpi3mr]
[   24.602366] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3083:32
[   24.602389] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.602430]  mpi3mr_sas_host_refresh+0x3b9/0x3e0 [mpi3mr]
[   24.602448]  mpi3mr_report_tgtdev_to_sas_transport+0x69/0x3e0 [mpi3mr]
[   24.602468]  mpi3mr_report_tgtdev_to_host.isra.0+0x1dc/0x250 [mpi3mr]
[   24.602485]  mpi3mr_fwevt_worker+0x7b2/0x1440 [mpi3mr]
[   24.602593] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3087:38
[   24.602611] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.602649]  mpi3mr_sas_host_refresh+0x3a0/0x3e0 [mpi3mr]
[   24.602668]  mpi3mr_report_tgtdev_to_sas_transport+0x69/0x3e0 [mpi3mr]
[   24.602687]  mpi3mr_report_tgtdev_to_host.isra.0+0x1dc/0x250 [mpi3mr]
[   24.602705]  mpi3mr_fwevt_worker+0x7b2/0x1440 [mpi3mr]
[   24.602812] UBSAN: array-index-out-of-bounds in mpi3mr_transport.c:3105:21
[   24.602827] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.602865]  mpi3mr_sas_host_refresh+0x37a/0x3e0 [mpi3mr]
[   24.602883]  mpi3mr_report_tgtdev_to_sas_transport+0x69/0x3e0 [mpi3mr]
[   24.602902]  mpi3mr_report_tgtdev_to_host.isra.0+0x1dc/0x250 [mpi3mr]
[   24.602918]  mpi3mr_fwevt_worker+0x7b2/0x1440 [mpi3mr]
[   24.610155]  end_device-0:0:0: mpi3mr_sas_port_add: added:
handle(0x0002), sas_address(0x5002538073479162)
[   24.616418]  end_device-0:0:1: mpi3mr_sas_port_add: added:
handle(0x0003), sas_address(0x5000cca2708a13c1)
[   24.623135]  end_device-0:0:2: mpi3mr_sas_port_add: added:
handle(0x0004), sas_address(0x5000cca2530eb471)
[   24.630940]  end_device-0:0:3: mpi3mr_sas_port_add: added:
handle(0x0005), sas_address(0x5002538073479292)
[   24.638042]  end_device-0:0:4: mpi3mr_sas_port_add: added:
handle(0x0006), sas_address(0x5002538073479072)
[   24.645612]  end_device-0:0:5: mpi3mr_sas_port_add: added:
handle(0x0007), sas_address(0x5002538073479012)
[   24.652827]  end_device-0:0:6: mpi3mr_sas_port_add: added:
handle(0x0008), sas_address(0x500253807347d062)
[   24.658932]  end_device-0:0:7: mpi3mr_sas_port_add: added:
handle(0x0009), sas_address(0x5000cca2530ca4cd)
[   24.661187] UBSAN: shift-out-of-bounds in mpi3mr_transport.c:1095:31
[   24.661197] Workqueue: mpi3mr0_fwevt_wrkr mpi3mr_fwevt_worker [mpi3mr]
[   24.661220]  mpi3mr_sas_port_add.cold+0x4c/0x64 [mpi3mr]
[   24.661227]  ? mpi3mr_set_identify+0x8a/0x200 [mpi3mr]
[   24.661234]  ? mpi3mr_update_links+0x143/0x240 [mpi3mr]
[   24.661241]  mpi3mr_report_tgtdev_to_sas_transport+0x1c7/0x3e0 [mpi3mr]
[   24.661247]  mpi3mr_report_tgtdev_to_host.isra.0+0x1dc/0x250 [mpi3mr]
[   24.661252]  mpi3mr_fwevt_worker+0x7b2/0x1440 [mpi3mr]
[   24.668188]  end_device-0:0:8: mpi3mr_sas_port_add: added:
handle(0x000a), sas_address(0x500e004aabcd337e)

Regards,

Ilya Khomyakov
khomyakovilya@xxxxxxxxx
# Technical report: incorrect SAS PHY link-rate reporting and undefined behavior in `mpi3mr`

## 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.

Attachment: 0002-scsi-mpi3mr-make-SAS-port-PHY-masks-64-bit-safe.patch
Description: Binary data

Attachment: 0003-scsi-mpi3mr-use-flexible-arrays-for-SAS-IO-Unit-PHY-.patch
Description: Binary data