From nobody Tue Oct 27 08:54:31 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4jDPSX711jz6xCbM for ; Tue, 27 Oct 2026 08:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4jDPSX5yztz4lZQ for ; Tue, 27 Oct 2026 08:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793091276; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ZssjFQyqfW8UGqOJkaDoWJaBqXl9Gbq/FKGwuf7LOF0=; b=VQIFTa1C8grQaio2ZhU2VyS3q0dMAkaSAoWjabMBHlygbbEvhnpO75un/bxMYe0b9yXIq1 59ZPShirA3An5b81+DWxjVeXPjZl7DGRjRIhNuQ0zNf1XNIb0EDeQFaK49pNCTXQZOsUsb LgBn2ioytm0DwR9yF9kbJjOojqLAUvMv6zZUQv9RAc1N9IJbN/6VMqM+n5cLxTCeOfV16U hWaUIOWdFbLkE7KTS5CER0HNqxj55U36T+ossOFeYrrobVB/JcpfGIHpzHz/yYNhXAYYRn GYFgDbMU677s4L92buUBcyvyyjXiKfVzHVdS0QRX4XrwBi3sTBCwVoQWvyxNqQ== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1793091276; b=tZIreikeyRqhEscRK1hGI+sZ73Y3y2fH+8C1Q8PyvhN286VKzHlHqF5J75nBURR/rex4Pf FXQPSw7M0BpavJSIO0S1cSg/cZldNgk0fZ024HJdS4h0j52Kn0P08WXRdJkFaAB9mjmUr4 yqttGmvcv3Q6Ttl+cghDiqFeFHPEw8OAwXPY5NAIbiii3Iky6/DMwFeA4/hViKtV5tDOB8 hUbS7tF8G0tWA03/wCEviXsC0BtDMeQd1dIgkgy8Uy3AuROYCYx6t3y6bV3K6XtyBBRlCV x4Lre3xiP+3uQ6QEuL4+WgjPX7lA6f1TxXao4FgCAfvTTQ9lImibtp6RGcAfZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793091276; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ZssjFQyqfW8UGqOJkaDoWJaBqXl9Gbq/FKGwuf7LOF0=; b=uiSyULJO37qfEo5sYy5IJil9giegowfNmCszrKu/dJE6mvLUHY7DaAXNyM7o2ocqOUWjKJ MhZW+r2ZUB6NfGxCbSEq6TB4Z2xMtVsWVEMI0zqgQilXhMyGEat9DFu32JCBby8vQ67G+p 2PIHWr/ocn7xwernUoR0Lgmt42rPnFnFa9xx/a58PDiUPv1Tz40PCiTlltTzdXe+l2+M7V v0nvPqDZT5M/xO4QzMKPhXckL0Oz+mx+gCnJYjN+pUF2HZPgyFaRMgk87Z48SLhbwauuBr 6Tb04nWBtMHnZ0awUm7i2s9P5nz2SroOiStN6hInQecbWU6ocuCW1Ym2cjjPYA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4jDPSX5P9KzDpt for ; Tue, 27 Oct 2026 08:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 45b1a by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Tue, 27 Oct 2026 08:54:31 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Ram Kishore Vegesna Subject: git: 26081296b244 - main - ocs_fc: Harden ioctl, XRI lookup and FCP command validation List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: Commit messages for all branches of the src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: ram X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 26081296b244a979732f2bd09e6def256e3d7bd4 Auto-Submitted: auto-generated Date: Tue, 27 Oct 2026 08:54:31 +0000 Message-Id: <6ae066c7.45b1a.74ac849d@gitrepo.freebsd.org> The branch main has been updated by ram: URL: https://cgit.FreeBSD.org/src/commit/?id=26081296b244a979732f2bd09e6def256e3d7bd4 commit 26081296b244a979732f2bd09e6def256e3d7bd4 Author: Ram Kishore Vegesna AuthorDate: 2026-10-14 11:24:07 +0000 Commit: Ram Kishore Vegesna 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 #include #include +#include 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; }