Page MenuHomeFreeBSD

mpi3mr: Validate event bounds and tighten error handling
Needs ReviewPublic

Authored by chandrakanth.patil_broadcom.com on Sun, Oct 4, 2:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 9, 12:25 PM
Unknown Object (File)
Thu, Oct 8, 9:08 AM
Unknown Object (File)
Thu, Oct 8, 6:49 AM
Unknown Object (File)
Wed, Oct 7, 10:33 PM
Unknown Object (File)
Wed, Oct 7, 8:06 PM
Unknown Object (File)
Wed, Oct 7, 7:54 PM
Unknown Object (File)
Wed, Oct 7, 8:07 AM
Unknown Object (File)
Tue, Oct 6, 6:46 PM
Subscribers
None

Details

Summary

Validate firmware event payload lengths in top-half and bottom-half
event handlers for device status change, prepare-for-reset, energy pack
change, cable management, and device info change events to prevent buffer
over-reads on truncated or malformed firmware packets. Propagate validation
status from the device status top-half handler to prevent scheduling
bottom-half worker processing for invalid packets.

In addition, unlock the admin request spinlock when rejecting submissions
on unrecoverable controllers to avoid deadlocks. Synchronize event queue
teardown with the dedicated firmware event lock rather than the main driver
mutex to match worker queue locking, and add null checks for memory
allocations in CAM attach and bitmap scanning routines.

Test Plan
  • Clean build with WERROR=-Werror across FreeBSD 16, 15, and 14 with INVARIANTS/WITNESS enabled; git bisect verified.
  • Verified active I/O running on RAID and JBOD volumes with concurrent online controller resets.
  • Validated management application functionality (StorCLI2) including VD/JBOD creation and deletion.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/dev/mpi3mr/mpi3mr.c
4356

sys/dev/mpi3mr/mpi/mpi30_ioc.h: U8 EventDataLength; /* 0x10 */
Since this is a 1 byte feild, this conversion is wrong on big endian, here and below.

sys/dev/mpi3mr/mpi3mr.c
4356

sys/dev/mpi3mr/mpi/mpi30_ioc.h: U8 EventDataLength; /* 0x10 */
Since this is a 1 byte feild, this conversion is wrong on big endian, here and below.

Thanks. I will remove le16toh() across all EventDataLength checks in V2 patch

v2 Changes (Code Review Feedback):

  • Fixed big-endian conversion: removed le16toh() on EventDataLength across all event top-half handlers (device status change, prepare for reset, energy pack change, and cable management), as EventDataLength is an 8-bit (U8) field.