git: 26081296b244 - main - ocs_fc: Harden ioctl, XRI lookup and FCP command validation
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 27 Oct 2026 08:54:31 UTC
The branch main has been updated by ram:
URL: https://cgit.FreeBSD.org/src/commit/?id=26081296b244a979732f2bd09e6def256e3d7bd4
commit 26081296b244a979732f2bd09e6def256e3d7bd4
Author: Ram Kishore Vegesna <ram@FreeBSD.org>
AuthorDate: 2026-10-14 11:24:07 +0000
Commit: Ram Kishore Vegesna <ram@FreeBSD.org>
CommitDate: 2026-10-27 08:51:38 +0000
ocs_fc: Harden ioctl, XRI lookup and FCP command validation
Fix several issues found by security review of the driver.
ioctl:
- Require PRIV_DRIVER for all ocs ioctls and create the device node
with mode 0600, group wheel.
- Return the error from ocs_process_sli_config() to the caller.
Previously only ENXIO was propagated, so a failed validation or a
failed copyin() still issued the mailbox command, in the latter case
with a descriptor pointing at freed memory.
- Reject COMMON_WRITE_OBJECT when in_bytes exceeds
desired_write_length, and cap WRITE_OBJECT, READ_OBJECT,
READ_OBJECT_LIST and non-embedded mailbox buffers at
OCS_MAX_DMA_ALLOC, returning EINVAL.
- Use a 64-bit size for the non-embedded mailbox buffer to avoid
truncation of a user supplied length.
- Refuse to copy out more than the size of the DMA buffer allocated
for the command.
- Range check the BAR offset in the ECD helper ioctl.
HW:
- Range check the XRI in ocs_hw_io_lookup() and return NULL when it is
out of range. Handle a NULL return in the auto xfer ready command
and data paths (returning the RQ buffers in the former), and in
ocs_hw_reclaim_xri().
- Make ocs_ref_get_unless_zero() atomic.
FCP:
- additional_fcp_cdb_length is in 4-byte words, but
ocs_validate_fcp_cmd() added it to the expected payload length as
bytes, so the length check under-validated the received frame.
- Drop commands with a CDB longer than CAM_MAX_CDBLEN instead of
passing a truncated CDB to CAM.
MFC after: 1 week
Sponsored by: Broadcom
Reviewed by: ken
---
sys/dev/ocs_fc/ocs_cam.c | 2 ++
sys/dev/ocs_fc/ocs_hw.c | 11 +++++++-
sys/dev/ocs_fc/ocs_hw_queues.c | 11 ++++++++
sys/dev/ocs_fc/ocs_ioctl.c | 59 +++++++++++++++++++++++++++++++++++++-----
sys/dev/ocs_fc/ocs_os.h | 11 +++++---
sys/dev/ocs_fc/ocs_unsol.c | 15 ++++++++++-
6 files changed, 97 insertions(+), 12 deletions(-)
diff --git a/sys/dev/ocs_fc/ocs_cam.c b/sys/dev/ocs_fc/ocs_cam.c
index 772e7ca1c383..46b1ca8a8e2a 100644
--- a/sys/dev/ocs_fc/ocs_cam.c
+++ b/sys/dev/ocs_fc/ocs_cam.c
@@ -607,6 +607,8 @@ int32_t ocs_scsi_recv_cmd(ocs_io_t *io, uint64_t lun, uint8_t *cdb,
atio->priority = (flags & OCS_SCSI_PRIORITY_MASK) >>
OCS_SCSI_PRIORITY_SHIFT;
+ if (cdb_len > sizeof(atio->cdb_io.cdb_bytes))
+ cdb_len = sizeof(atio->cdb_io.cdb_bytes);
atio->cdb_len = cdb_len;
ocs_memcpy(atio->cdb_io.cdb_bytes, cdb, cdb_len);
diff --git a/sys/dev/ocs_fc/ocs_hw.c b/sys/dev/ocs_fc/ocs_hw.c
index f4935db8f321..8041f94810cd 100644
--- a/sys/dev/ocs_fc/ocs_hw.c
+++ b/sys/dev/ocs_fc/ocs_hw.c
@@ -9736,7 +9736,14 @@ ocs_hw_io_t *
ocs_hw_io_lookup(ocs_hw_t *hw, uint32_t xri)
{
uint32_t ioindex;
- ioindex = xri - hw->sli.config.extent[SLI_RSRC_FCOE_XRI].base[0];
+ uint32_t base = hw->sli.config.extent[SLI_RSRC_FCOE_XRI].base[0];
+
+ if (xri < base || (xri - base) >= hw->config.n_io) {
+ ocs_log_err(hw->os, "xri %#x out of range (base=%#x n_io=%u)\n",
+ xri, base, hw->config.n_io);
+ return NULL;
+ }
+ ioindex = xri - base;
return hw->io[ioindex];
}
@@ -11346,6 +11353,8 @@ ocs_hw_reclaim_xri(ocs_hw_t *hw, uint16_t xri_base, uint16_t xri_count)
for (i = 0; i < xri_count; i++) {
io = ocs_hw_io_lookup(hw, xri_base + i);
+ if (io == NULL)
+ continue;
/*
* if this is an auto xfer rdy XRI, then we need to release any
diff --git a/sys/dev/ocs_fc/ocs_hw_queues.c b/sys/dev/ocs_fc/ocs_hw_queues.c
index 72534e576dd9..06456275e4c0 100644
--- a/sys/dev/ocs_fc/ocs_hw_queues.c
+++ b/sys/dev/ocs_fc/ocs_hw_queues.c
@@ -1873,6 +1873,12 @@ ocs_hw_rqpair_process_auto_xfr_rdy_cmd(ocs_hw_t *hw, hw_cq_t *cq, uint8_t *cqe)
fc_header_t *fc_hdr = seq->header->dma.virt;
seq->hio = ocs_hw_io_lookup(hw, seq->xri);
+ if (seq->hio == NULL) {
+ ocs_log_err(hw->os, "auto_xrdy CQE, invalid xri=%#x\n",
+ seq->xri);
+ ocs_hw_rqpair_sequence_free(hw, seq);
+ return -1;
+ }
ocs_lock(&seq->hio->axr_lock);
axr_lock_taken = 1;
@@ -1971,6 +1977,11 @@ ocs_hw_rqpair_process_auto_xfr_rdy_data(ocs_hw_t *hw, hw_cq_t *cq, uint8_t *cqe)
#endif
/* Look up the IO */
io = ocs_hw_io_lookup(hw, opt_wr->xri);
+ if (io == NULL) {
+ ocs_log_err(hw->os, "auto_xrdy data CQE for invalid xri=%#x\n",
+ opt_wr->xri);
+ return -1;
+ }
ocs_lock(&io->axr_lock);
buf = io->axr_buf;
buf->data_cqe = 1;
diff --git a/sys/dev/ocs_fc/ocs_ioctl.c b/sys/dev/ocs_fc/ocs_ioctl.c
index d3cea434b2be..a978f1084452 100644
--- a/sys/dev/ocs_fc/ocs_ioctl.c
+++ b/sys/dev/ocs_fc/ocs_ioctl.c
@@ -39,6 +39,7 @@
#include <sys/systm.h>
#include <sys/linker.h>
#include <sys/firmware.h>
+#include <sys/priv.h>
static d_open_t ocs_open;
static d_close_t ocs_close;
@@ -97,6 +98,11 @@ ocs_process_sli_config (ocs_t *ocs, ocs_ioctl_elxu_mbox_t *mcmd, ocs_dma_t *dma)
sli4_req_common_read_object_t *rdobj =
(sli4_req_common_read_object_t *)sli_config->payload.embed;
+ if (mcmd->out_bytes > OCS_MAX_DMA_ALLOC) {
+ device_printf(ocs->dev, "%s: COMMON_READ_OBJECT - %llu exceeds max\n",
+ __func__, (unsigned long long)mcmd->out_bytes);
+ return EINVAL;
+ }
if (ocs_dma_alloc(ocs, dma, mcmd->out_bytes, 4096)) {
device_printf(ocs->dev, "%s: COMMON_READ_OBJECT - %lld allocation failed\n",
__func__, (unsigned long long)mcmd->out_bytes);
@@ -116,6 +122,17 @@ ocs_process_sli_config (ocs_t *ocs, ocs_ioctl_elxu_mbox_t *mcmd, ocs_dma_t *dma)
sli4_req_common_write_object_t *wrobj =
(sli4_req_common_write_object_t *)sli_config->payload.embed;
+ if (mcmd->in_bytes > wrobj->desired_write_length) {
+ device_printf(ocs->dev, "%s: COMMON_WRITE_OBJECT - in_bytes %llu exceeds desired_write_length %u\n",
+ __func__, (unsigned long long)mcmd->in_bytes,
+ wrobj->desired_write_length);
+ return EINVAL;
+ }
+ if (wrobj->desired_write_length > OCS_MAX_DMA_ALLOC) {
+ device_printf(ocs->dev, "%s: COMMON_WRITE_OBJECT - desired_write_length %u exceeds max\n",
+ __func__, wrobj->desired_write_length);
+ return EINVAL;
+ }
if (ocs_dma_alloc(ocs, dma, wrobj->desired_write_length, 4096)) {
device_printf(ocs->dev, "%s: COMMON_WRITE_OBJECT - %d allocation failed\n",
__func__, wrobj->desired_write_length);
@@ -144,6 +161,11 @@ ocs_process_sli_config (ocs_t *ocs, ocs_ioctl_elxu_mbox_t *mcmd, ocs_dma_t *dma)
sli4_req_common_read_object_list_t *rdobj =
(sli4_req_common_read_object_list_t *)sli_config->payload.embed;
+ if (mcmd->out_bytes > OCS_MAX_DMA_ALLOC) {
+ device_printf(ocs->dev, "%s: COMMON_READ_OBJECT_LIST - %llu exceeds max\n",
+ __func__, (unsigned long long)mcmd->out_bytes);
+ return EINVAL;
+ }
if (ocs_dma_alloc(ocs, dma, mcmd->out_bytes, 4096)) {
device_printf(ocs->dev, "%s: COMMON_READ_OBJECT_LIST - %lld allocation failed\n",
__func__,(unsigned long long) mcmd->out_bytes);
@@ -170,10 +192,15 @@ ocs_process_sli_config (ocs_t *ocs, ocs_ioctl_elxu_mbox_t *mcmd, ocs_dma_t *dma)
break;
}
} else {
- uint32_t max_bytes = max(mcmd->in_bytes, mcmd->out_bytes);
+ uint64_t max_bytes = max(mcmd->in_bytes, mcmd->out_bytes);
+ if (max_bytes > OCS_MAX_DMA_ALLOC) {
+ device_printf(ocs->dev, "%s: non-embedded - %llu exceeds max\n",
+ __func__, (unsigned long long)max_bytes);
+ return EINVAL;
+ }
if (ocs_dma_alloc(ocs, dma, max_bytes, 4096)) {
- device_printf(ocs->dev, "%s: non-embedded - %u allocation failed\n",
- __func__, max_bytes);
+ device_printf(ocs->dev, "%s: non-embedded - %llu allocation failed\n",
+ __func__, (unsigned long long)max_bytes);
return ENXIO;
}
@@ -210,8 +237,17 @@ ocs_process_mbx_ioctl(ocs_t *ocs, ocs_ioctl_elxu_mbox_t *mcmd)
switch(((sli4_mbox_command_header_t *)mcmd->payload)->command) {
case SLI4_MBOX_COMMAND_SLI_CONFIG:
- if (ENXIO == ocs_process_sli_config(ocs, mcmd, &dma))
- return ENXIO;
+ error = ocs_process_sli_config(ocs, mcmd, &dma);
+ if (error != 0)
+ goto no_support;
+ /* never copy out more than was allocated for the command */
+ if (mcmd->out_bytes && dma.virt && mcmd->out_bytes > dma.size) {
+ device_printf(ocs->dev, "%s: out_bytes %llu exceeds buffer size %zu\n",
+ __func__, (unsigned long long)mcmd->out_bytes,
+ dma.size);
+ error = EINVAL;
+ goto no_support;
+ }
break;
case SLI4_MBOX_COMMAND_READ_REV:
@@ -299,6 +335,12 @@ 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) >
+ 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;
default:
break;
@@ -359,6 +401,11 @@ ocs_ioctl(struct cdev *cdev, u_long cmd, caddr_t addr, int flag, struct thread *
struct ocs_softc *ocs = cdev->si_drv1;
device_t dev = ocs->dev;
+ status = priv_check(td, PRIV_DRIVER);
+ if (status) {
+ return (status);
+ }
+
switch (cmd) {
case OCS_IOCTL_CMD_ELXU_MBOX: {
/* "copyin" done by kernel; thus, just dereference addr */
@@ -1212,7 +1259,7 @@ ocs_debug_attach(void *os)
}
unit = device_get_unit(ocs->dev);
- ocs->cdev = make_dev(&ocs_cdevsw, unit, UID_ROOT, GID_OPERATOR, 0640,
+ ocs->cdev = make_dev(&ocs_cdevsw, unit, UID_ROOT, GID_WHEEL, 0600,
"ocs%d", unit);
if (ocs->cdev) {
ocs->cdev->si_drv1 = ocs;
diff --git a/sys/dev/ocs_fc/ocs_os.h b/sys/dev/ocs_fc/ocs_os.h
index 55ee98b3cc58..2fadafdb3e01 100644
--- a/sys/dev/ocs_fc/ocs_os.h
+++ b/sys/dev/ocs_fc/ocs_os.h
@@ -1325,11 +1325,14 @@ ocs_ref_get(ocs_ref_t *ref)
static inline uint32_t
ocs_ref_get_unless_zero(ocs_ref_t *ref)
{
- uint32_t rc = 0;
+ uint32_t rc;
+
rc = ocs_atomic_read(&ref->count);
- if (rc != 0) {
- ocs_atomic_add_return(&ref->count, 1);
- }
+ do {
+ if (rc == 0)
+ return 0;
+ } while (!atomic_fcmpset_32(&ref->count, &rc, rc + 1));
+
return rc;
}
diff --git a/sys/dev/ocs_fc/ocs_unsol.c b/sys/dev/ocs_fc/ocs_unsol.c
index 89cc015dc54b..42f74425dd82 100644
--- a/sys/dev/ocs_fc/ocs_unsol.c
+++ b/sys/dev/ocs_fc/ocs_unsol.c
@@ -788,8 +788,11 @@ static int32_t
ocs_validate_fcp_cmd(ocs_t *ocs, ocs_hw_sequence_t *seq)
{
size_t exp_payload_len = 0;
+ size_t cdb_len;
fcp_cmnd_iu_t *cmnd = seq->payload->dma.virt;
- exp_payload_len = sizeof(fcp_cmnd_iu_t) - 16 + cmnd->additional_fcp_cdb_length;
+ /* additional_fcp_cdb_length is in 4-byte words */
+ exp_payload_len = sizeof(fcp_cmnd_iu_t) - 16 +
+ cmnd->additional_fcp_cdb_length * sizeof(uint32_t);
/*
* If we received less than FCP_CMND_IU bytes, assume that the frame is
@@ -803,6 +806,16 @@ ocs_validate_fcp_cmd(ocs_t *ocs, ocs_hw_sequence_t *seq)
exp_payload_len);
return -1;
}
+
+ /* CAM cannot carry a CDB longer than CAM_MAX_CDBLEN; do not truncate */
+ cdb_len = sizeof(cmnd->fcp_cdb) +
+ cmnd->additional_fcp_cdb_length * sizeof(uint32_t);
+ if (cmnd->task_management_flags == 0 && cdb_len > CAM_MAX_CDBLEN) {
+ fc_header_t *fchdr = seq->header->dma.virt;
+ ocs_log_debug(ocs, "dropping ox_id %04x with unsupported CDB length (%zd > %d)\n",
+ ocs_be16toh(fchdr->ox_id), cdb_len, CAM_MAX_CDBLEN);
+ return -1;
+ }
return 0;
}