[PATCH] scsi: mpt3sas: validate variable event array spans

Pengpeng Hou posted 1 patch 2 days, 18 hours ago
drivers/scsi/mpt3sas/mpt3sas_base.c | 71 +++++++++++++++++++++++++++++++++++--
1 file changed, 69 insertions(+), 2 deletions(-)
[PATCH] scsi: mpt3sas: validate variable event array spans
Posted by Pengpeng Hou 2 days, 18 hours ago
SAS topology, PCIe topology and IR configuration events contain flexible
arrays whose element counts are supplied by firmware. Their interrupt-time
handlers use NumEntries or NumElements without first checking that the
corresponding array fits in EventData.

Validate only these three variable-array event types before dispatch. Check
that MsgLength is within the allocated reply frame, that EventDataLength is
within MsgLength, and that the counted array fits in EventData.

Malformed events still follow the existing ACK path before they are
dropped, so validation does not leave an acknowledged event pending in
firmware. Other event types retain their existing behavior.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 71 +++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 79052f2accbd..2a9ea81d1e2b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1333,6 +1333,65 @@ _base_display_event_data(struct MPT3SAS_ADAPTER *ioc,
 	ioc_info(ioc, "%s\n", desc);
 }
 
+static bool
+_base_variable_event_data_valid(struct MPT3SAS_ADAPTER *ioc,
+				Mpi2EventNotificationReply_t *mpi_reply)
+{
+	const size_t event_offset = offsetof(Mpi2EventNotificationReply_t,
+					     EventData);
+	const void *event_data = mpi_reply->EventData;
+	size_t event_data_len;
+	size_t reply_len;
+	u16 event;
+
+	event = le16_to_cpu(mpi_reply->Event);
+	switch (event) {
+	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST:
+	case MPI2_EVENT_PCIE_TOPOLOGY_CHANGE_LIST:
+	case MPI2_EVENT_IR_CONFIGURATION_CHANGE_LIST:
+		break;
+	default:
+		return true;
+	}
+
+	reply_len = mpi_reply->MsgLength * 4;
+	if (reply_len < event_offset || reply_len > ioc->reply_sz)
+		return false;
+
+	event_data_len = le16_to_cpu(mpi_reply->EventDataLength) * 4;
+	if (event_data_len > reply_len - event_offset)
+		return false;
+
+	switch (event) {
+	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST: {
+		const Mpi2EventDataSasTopologyChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi2EventDataSasTopologyChangeList_t,
+					    PHY);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, PHY, data->NumEntries) <= event_data_len;
+	}
+	case MPI2_EVENT_PCIE_TOPOLOGY_CHANGE_LIST: {
+		const Mpi26EventDataPCIeTopologyChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi26EventDataPCIeTopologyChangeList_t,
+					    PortEntry);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, PortEntry, data->NumEntries) <= event_data_len;
+	}
+	case MPI2_EVENT_IR_CONFIGURATION_CHANGE_LIST: {
+		const Mpi2EventDataIrConfigChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi2EventDataIrConfigChangeList_t,
+					    ConfigElement);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, ConfigElement, data->NumElements) <= event_data_len;
+	}
+	default:
+		return true;
+	}
+}
+
 /**
  * _base_sas_log_info - verbose translation of firmware log info
  * @ioc: per adapter object
@@ -1482,8 +1541,9 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 {
 	Mpi2EventNotificationReply_t *mpi_reply;
 	Mpi2EventAckRequest_t *ack_request;
-	u16 smid;
 	struct _event_ack_list *delayed_event_ack;
+	bool event_data_valid;
+	u16 smid;
 
 	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
 	if (!mpi_reply)
@@ -1491,7 +1551,12 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	if (mpi_reply->Function != MPI2_FUNCTION_EVENT_NOTIFICATION)
 		return 1;
 
-	_base_display_event_data(ioc, mpi_reply);
+	event_data_valid = _base_variable_event_data_valid(ioc, mpi_reply);
+	if (event_data_valid)
+		_base_display_event_data(ioc, mpi_reply);
+	else
+		ioc_warn(ioc, "dropping malformed event 0x%04x\n",
+			 le16_to_cpu(mpi_reply->Event));
 
 	if (!(mpi_reply->AckRequired & MPI2_EVENT_NOTIFICATION_ACK_REQUIRED))
 		goto out;
@@ -1521,6 +1586,8 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	ioc->put_smid_default(ioc, smid);
 
  out:
+	if (!event_data_valid)
+		return 1;
 
 	/* scsih callback handler */
 	mpt3sas_scsih_event_callback(ioc, msix_index, reply);
-- 
2.50.1 (Apple Git-155)
Re: [PATCH] scsi: mpt3sas: validate variable event array spans
Posted by James Bottomley 2 days, 11 hours ago
On Wed, 2026-07-22 at 12:12 +0800, Pengpeng Hou wrote:
> SAS topology, PCIe topology and IR configuration events contain
> flexible arrays whose element counts are supplied by firmware. Their
> interrupt-time handlers use NumEntries or NumElements without first
> checking that the corresponding array fits in EventData.

Please explain why we might need to check this.  The device is a fat
firmware one meaning the driver is usually set up with the correctly
matching limits to the ones the firmware returns.

Regards,

James