Page MenuHomeFreeBSD

mpi3mr: Enforce frame alignment and bounds checks on reply and sense buffers
Needs ReviewPublic

Authored by chandrakanth.patil_broadcom.com on Sun, Oct 4, 2:18 PM.
Tags
None
Referenced Files
F174916168: D60338.diff
Tue, Oct 6, 11:06 PM
F174892134: D60338.diff
Tue, Oct 6, 7:33 PM
F174829357: D60338.diff
Tue, Oct 6, 7:16 AM
Unknown Object (File)
Mon, Oct 5, 3:46 AM
Subscribers
None

Details

Summary

When the controller posts command completions or sense data, firmware
provides physical DMA addresses that the driver translates into virtual
addresses within its pre-allocated reply and sense buffer pools.

Previously, these address translation helpers lacked complete validation:

  1. The reply buffer lookup checked only the starting physical address without verifying that the full frame size fit within the pool, and did not check whether the address aligned to the fixed reply frame stride.
  2. The sense buffer lookup performed no bounds checking whatsoever, blindly computing an offset from the pool base.

If firmware reported a corrupted address or an unaligned offset, the driver
computed an invalid virtual pointer, leading to memory access outside the
allocated buffer pools.

Validate that physical addresses for both reply and sense buffers fall
strictly within their respective pool bounds, ensure the complete frame size
fits within the pool, and enforce exact alignment to frame size boundaries.

Test Plan
  • Clean build with WERROR=-Werror across FreeBSD 16, 15, and 14 with INVARIANTS/WITNESS enabled; git bisect verified.
  • Tested high-throughput I/O and heavy SCSI error injection (CHECK CONDITION with sense data) on SAS4116/SAS5116 controllers.
  • Verified that reply and sense buffer physical-to-virtual address translations strictly enforce frame alignment and bounds checks without invalid pointer calculations.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/dev/mpi3mr/mpi3mr.c
250

Isn't this unsafe now if both phy_addr is near the top of 64-bit address space? Adding the reply_sz could overflow, leading the result ot be less than this max address?

sys/dev/mpi3mr/mpi3mr.c
250

Isn't this unsafe now if both phy_addr is near the top of 64-bit address space? Adding the reply_sz could overflow, leading the result ot be less than this max address?

Thanks. I will update this to subtract from the upper bound in V2 patch to prevent 64-bit integer overflow: phys_addr > (sc->reply_buf_dma_max_address - sc->reply_sz)

I will apply the same fix to the sense buffer lookup as well.