git: 53b576bd81a4 - main - if_bnxt: Pass HWRM cmd response to apps even if FW fails it

From: Sumit Saxena <ssaxena_at_FreeBSD.org>
Date: Fri, 09 Oct 2026 10:12:44 UTC
The branch main has been updated by ssaxena:

URL: https://cgit.FreeBSD.org/src/commit/?id=53b576bd81a4fb5c38b952e74ff2f806569bc8ac

commit 53b576bd81a4fb5c38b952e74ff2f806569bc8ac
Author:     Sumit Saxena <sumit.saxena@broadcom.com>
AuthorDate: 2026-07-31 15:48:07 +0000
Commit:     Sumit Saxena <ssaxena@FreeBSD.org>
CommitDate: 2026-10-09 08:24:44 +0000

    if_bnxt: Pass HWRM cmd response to apps even if FW fails it
    
    The driver was not passing the HWRM command response to management
    apps when the firmware failed the command, which is not what apps
    expect. Copy the response out whenever firmware populated one
    (resp_len != 0), regardless of the command's overall return code, and
    stop bailing out of the mgmt ioctl path early on a failed passthrough
    so DMA'd indirect data and the response still get copied to
    userspace.
    
    Reviewed by:    gallatin
    MFC after:      2 weeks
    Sponsored by:   Broadcom Inc.
    Differential Revision:  https://reviews.freebsd.org/D58609
---
 sys/dev/bnxt/bnxt_en/bnxt_hwrm.c | 17 ++++++++++++++---
 sys/dev/bnxt/bnxt_en/bnxt_mgmt.c | 16 ++++++++++++++--
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/sys/dev/bnxt/bnxt_en/bnxt_hwrm.c b/sys/dev/bnxt/bnxt_en/bnxt_hwrm.c
index 57af44ec38c5..680bb593c4b4 100644
--- a/sys/dev/bnxt/bnxt_en/bnxt_hwrm.c
+++ b/sys/dev/bnxt/bnxt_en/bnxt_hwrm.c
@@ -876,7 +876,7 @@ bnxt_hwrm_passthrough(struct bnxt_softc *softc, void *req, uint32_t req_len,
 		void *resp, uint32_t resp_len, uint32_t app_timeout)
 {
 	int rc = 0;
-	void *output = (void *)softc->hwrm_cmd_resp.idi_vaddr;
+	struct hwrm_err_output *output = (void *)softc->hwrm_cmd_resp.idi_vaddr;
 	struct input *input = req;
 	uint32_t old_timeo;
 
@@ -889,13 +889,24 @@ bnxt_hwrm_passthrough(struct bnxt_softc *softc, void *req, uint32_t req_len,
 		softc->hwrm_cmd_timeo = max(app_timeout, softc->hwrm_cmd_timeo);
 	rc = _hwrm_send_message(softc, req, req_len);
 	softc->hwrm_cmd_timeo = old_timeo;
+
+	/*
+	 * On ETIMEDOUT the shared resp buffer may still hold a stale, unrelated
+	 * response, so skip the copy; otherwise clamp to resp_len and PAGE_SIZE.
+	 */
+	if (rc != ETIMEDOUT) {
+		uint32_t copy_len = min(resp_len, output->resp_len);
+
+		copy_len = min(copy_len, PAGE_SIZE);
+		if (copy_len)
+			memcpy(resp, output, copy_len);
+	}
+
 	if (rc) {
 		device_printf(softc->dev, "%s: %s command failed with rc: 0x%x\n",
 			      __FUNCTION__, GET_HWRM_REQ_TYPE(input->req_type), rc);
 		goto fail;
 	}
-
-	memcpy(resp, output, resp_len);
 fail:
 	BNXT_HWRM_UNLOCK(softc);
 	return rc;
diff --git a/sys/dev/bnxt/bnxt_en/bnxt_mgmt.c b/sys/dev/bnxt/bnxt_en/bnxt_mgmt.c
index edf50ac1432f..7a200df0fda4 100644
--- a/sys/dev/bnxt/bnxt_en/bnxt_mgmt.c
+++ b/sys/dev/bnxt/bnxt_en/bnxt_mgmt.c
@@ -592,6 +592,20 @@ bnxt_mgmt_process_hwrm(struct cdev *dev, u_long cmd, caddr_t data,
 			ret = -EFAULT;
 			goto end;
 		}
+
+		/*
+		 * TOCTOU: this second copyin re-reads len_req/len_resp from
+		 * userspace, so require them to match msg_temp's already-validated
+		 * values to prevent overflowing the buffers sized from those.
+		 */
+		if (msg2->len_req != msg_temp.len_req ||
+		    msg2->len_resp != msg_temp.len_resp) {
+			device_printf(softc->dev, "%s:%d len_req/len_resp "
+				      "mismatch on second copy\n",
+				      __func__, __LINE__);
+			ret = -EINVAL;
+			goto end;
+		}
 		msg = msg2;
 
 		for (i = 0; i < num_ind; i++) {
@@ -635,8 +649,6 @@ bnxt_mgmt_process_hwrm(struct cdev *dev, u_long cmd, caddr_t data,
 	}
 
 	ret = bnxt_hwrm_passthrough(softc, req, msg->len_req, resp, msg->len_resp, msg->timeout);
-	if (ret)
-		goto end;
 
 	for (i = 0; i < num_ind; i++) {
 		if ((msg->dma[i].read_or_write)) {