git: 039a8e11f887 - main - ocs_fc: Fix config space bounds, double PRLI response and a stack overflow
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 27 Oct 2026 08:54:32 UTC
The branch main has been updated by ram:
URL: https://cgit.FreeBSD.org/src/commit/?id=039a8e11f887e359af1346b04e05180b9a49920c
commit 039a8e11f887e359af1346b04e05180b9a49920c
Author: Ram Kishore Vegesna <ram@FreeBSD.org>
AuthorDate: 2026-10-19 17:29:31 +0000
Commit: Ram Kishore Vegesna <ram@FreeBSD.org>
CommitDate: 2026-10-27 08:52:14 +0000
ocs_fc: Fix config space bounds, double PRLI response and a stack overflow
Fix three unrelated problems found by security review of the driver.
ECD helper ioctl:
- OCS_ECD_HELPER_CFG_READ* and OCS_ECD_HELPER_CFG_WRITE* passed a user
supplied offset straight to pci_read_config() and pci_write_config().
Reject accesses that extend past PCIE_REGMAX, so that a wide access
starting in range cannot run past the end of config space.
- Use the real access width (1, 2 or 4 bytes) of the command for both
the BAR and the config space checks, rather than assuming 4 bytes, so
that narrow accesses at the very end of the BAR or config space are
not rejected while wider ones remain bounded.
PRLI:
- When a PRLI carries an FC-4 type other than the one the driver
supports, ocs_d_send_prli_rsp() sent an LS_RJT and started node
shutdown, but then fell through and sent a second response on the same
IO (another LS_RJT, or a PRLI LS_ACC when the initiator is accepted),
followed by a conflicting transition to __ocs_d_device_ready. The two
responses posted two WQEs for one IO and caused a double ocs_ref_put(),
a use after free of the ELS IO. Return after dispatching the LS_RJT so
that exactly one response is sent.
- __ocs_d_port_logged_in calls ocs_d_send_prli_rsp() without checking
the FC-4 type, so any port that has completed a PLOGI can trigger this
with a single PRLI, whether or not target mode is enabled.
Management:
- The active profile id is a uint32_t that is formatted as "0x%02x"
with sprintf() into a buffer of MAX_PROFILE_LENGTH (5) bytes, which
overflows for any id above 0xff. Size the buffer for the longest
possible string ("0x" plus 8 hex digits and the terminating NUL) and
use ocs_snprintf().
- Use ocs_snprintf() for nv_wwpn as well, as nv_wwnn already does.
MFC after: 1 week
Sponsored by: Broadcom
Reviewed by: ken
---
sys/dev/ocs_fc/ocs_device.c | 2 ++
sys/dev/ocs_fc/ocs_ioctl.c | 42 +++++++++++++++++++++++++++++++++++++++++-
sys/dev/ocs_fc/ocs_mgmt.c | 8 +++++---
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/sys/dev/ocs_fc/ocs_device.c b/sys/dev/ocs_fc/ocs_device.c
index d9c283541d3c..4d85970df675 100644
--- a/sys/dev/ocs_fc/ocs_device.c
+++ b/sys/dev/ocs_fc/ocs_device.c
@@ -74,6 +74,8 @@ ocs_d_send_prli_rsp(ocs_io_t *io, uint16_t ox_id)
FC_EXPL_REQUEST_NOT_SUPPORTED, 0, NULL, NULL);
node->shutdown_reason = OCS_NODE_SHUTDOWN_DEFAULT;
ocs_node_transition(node, __ocs_d_initiate_shutdown, NULL);
+ /* Send only one response per PRLI; the LS_RJT owns the IO */
+ return;
}
/* If the back-end doesn't want to talk to this initiator, we send an LS_RJT */
diff --git a/sys/dev/ocs_fc/ocs_ioctl.c b/sys/dev/ocs_fc/ocs_ioctl.c
index a978f1084452..18dd3fbbac4c 100644
--- a/sys/dev/ocs_fc/ocs_ioctl.c
+++ b/sys/dev/ocs_fc/ocs_ioctl.c
@@ -41,6 +41,8 @@
#include <sys/firmware.h>
#include <sys/priv.h>
+#include <dev/pci/pcireg.h>
+
static d_open_t ocs_open;
static d_close_t ocs_close;
static d_ioctl_t ocs_ioctl;
@@ -299,6 +301,28 @@ no_support:
return error;
}
+/**
+ * @brief return the access width in bytes of an ECD helper BAR/config command
+ */
+static uint32_t
+ocs_ecd_helper_width(ocs_ecd_helper_cmd_t cmd)
+{
+ switch (cmd) {
+ case OCS_ECD_HELPER_BAR_READ8:
+ case OCS_ECD_HELPER_BAR_WRITE8:
+ case OCS_ECD_HELPER_CFG_READ8:
+ case OCS_ECD_HELPER_CFG_WRITE8:
+ return (sizeof(uint8_t));
+ case OCS_ECD_HELPER_BAR_READ16:
+ case OCS_ECD_HELPER_BAR_WRITE16:
+ case OCS_ECD_HELPER_CFG_READ16:
+ case OCS_ECD_HELPER_CFG_WRITE16:
+ return (sizeof(uint16_t));
+ default:
+ return (sizeof(uint32_t));
+ }
+}
+
/**
* @brief perform requested Elx CoreDump helper function
*
@@ -318,6 +342,7 @@ ocs_process_ecd_helper (ocs_t *ocs, ocs_ioctl_ecd_helper_t *req)
uint8_t v8;
uint16_t v16;
uint32_t v32;
+ uint32_t width;
/* Check the BAR read/write commands for valid bar */
switch(req->cmd) {
@@ -327,6 +352,7 @@ ocs_process_ecd_helper (ocs_t *ocs, ocs_ioctl_ecd_helper_t *req)
case OCS_ECD_HELPER_BAR_WRITE8:
case OCS_ECD_HELPER_BAR_WRITE16:
case OCS_ECD_HELPER_BAR_WRITE32:
+ width = ocs_ecd_helper_width(req->cmd);
if (req->bar >= PCI_MAX_BAR) {
device_printf(ocs->dev, "Error: bar %d out of range\n", req->bar);
return -EFAULT;
@@ -335,13 +361,27 @@ ocs_process_ecd_helper (ocs_t *ocs, ocs_ioctl_ecd_helper_t *req)
device_printf(ocs->dev, "Error: bar %d not defined\n", req->bar);
return -EFAULT;
}
- if ((uint64_t)req->offset + sizeof(uint32_t) >
+ if ((uint64_t)req->offset + width >
rman_get_size(ocs->reg[req->bar].res)) {
device_printf(ocs->dev, "Error: bar %d offset 0x%x out of range\n",
req->bar, req->offset);
return -EFAULT;
}
break;
+ case OCS_ECD_HELPER_CFG_READ8:
+ case OCS_ECD_HELPER_CFG_READ16:
+ case OCS_ECD_HELPER_CFG_READ32:
+ case OCS_ECD_HELPER_CFG_WRITE8:
+ case OCS_ECD_HELPER_CFG_WRITE16:
+ case OCS_ECD_HELPER_CFG_WRITE32:
+ width = ocs_ecd_helper_width(req->cmd);
+ if ((uint64_t)req->offset + width - 1 > PCIE_REGMAX) {
+ device_printf(ocs->dev,
+ "Error: config offset 0x%x out of range\n",
+ req->offset);
+ return -EFAULT;
+ }
+ break;
default:
break;
}
diff --git a/sys/dev/ocs_fc/ocs_mgmt.c b/sys/dev/ocs_fc/ocs_mgmt.c
index 5b7f6557c017..483e80ce649a 100644
--- a/sys/dev/ocs_fc/ocs_mgmt.c
+++ b/sys/dev/ocs_fc/ocs_mgmt.c
@@ -2076,7 +2076,8 @@ ocs_mgmt_get_active_profile_cb(int32_t status, uint32_t active_profile, void *ul
ocs_sem_v(&(result->semaphore));
}
-#define MAX_PROFILE_LENGTH 5
+/* "0x", up to 8 hex digits and a NUL for a uint32_t id */
+#define MAX_PROFILE_LENGTH 11
/**
* @brief Get active profile
@@ -2107,7 +2108,7 @@ get_active_profile(ocs_t *ocs, char *name, ocs_textbuf_t *textbuf)
}
if (result.status == 0) {
/* Success. */
- sprintf(result_string, "0x%02x", result.active_profile_id);
+ ocs_snprintf(result_string, sizeof(result_string), "0x%02x", result.active_profile_id);
ocs_mgmt_emit_string(textbuf, MGMT_MODE_RW, "active_profile", result_string);
} else {
ocs_log_test(ocs, "getting active profile status 0x%x\n", result.status);
@@ -2225,7 +2226,8 @@ get_nv_wwpn(ocs_t *ocs, char *name, ocs_textbuf_t *textbuf)
}
if (result.status == 0) {
/* Success. Copy wwpn from result struct to result string */
- sprintf(result_string, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x",
+ ocs_snprintf(result_string, sizeof(result_string),
+ "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x",
result.wwpn[0], result.wwpn[1], result.wwpn[2],
result.wwpn[3], result.wwpn[4], result.wwpn[5],
result.wwpn[6], result.wwpn[7]);