From nobody Thu Oct 29 23:37:15 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 4jG0y35D8sz6xr06 for ; Thu, 29 Oct 2026 23:37:15 +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 4jG0y32CYdz4sld for ; Thu, 29 Oct 2026 23:37:15 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793317035; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=mDqCgDj652iBFxFHObPDyh74CiF0q2neudPWa29VqjI=; b=lx2K8YhjK8m4F7qm9i3/+/y0KuqclxWdZSvOTjex0ompPFwwabvLQRzGbTfnHpTHBXq5mM eW2D4S6BFwME3Eyyzt7LGQ9W8lyAqcLmsYBDcXxM3kG86L4Q/ocQVy/KFeQ6N5gFutdTR5 vPShBHHOWiTCK7gotyCPDhxsg2kh9PDN5s/yMlgi5d90DTsGgVE8q/BjAtyOTw/wVExlIk XNMKtstgufpCubpb9UUTB0WT5IZuQxPYtK7qA1iJgb3Tndodu31zk83GKa7RJINFtG5nrK nN97qv7PSMxJlPyP2D/CdmHYMz1oft285kGqitkQKRnHzYtsHMVYZcqGX2a18Q== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1793317035; b=xRetHyHO5eenP6PpFeJLoo2ArfCW1EWbPasVPqWef6gTCL0cRJHZQMEKkayNdAMb/y/SlQ QKFnsz+drTqCiDZkbD8/tPKmPDSqmaRbDSnX0ZatOtEN7twlVYzDsHmiWtWe1QhfxHLQY/ yeXRzM+vhDIg79LUVWbBU6l6aMg4MvlGS36N0Lz0X8Lka5AU80vUXhjvFWfHK/VeYaPhTF IN0uU9ziESd4vfRQ0hnRWP9oTyE1ZHFakUud1b21f244uQ8gpiWN5BaSuxkqskQMlc8CX1 N/JfBRstKVjIp8CVMOnCeaXcSRk9k5FwhgLcQm507+AuiSmWvlfJ+iX2NVYOvg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793317035; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=mDqCgDj652iBFxFHObPDyh74CiF0q2neudPWa29VqjI=; b=apPyn35lxPvmgsjnLF9p87X5NzrsADFBtsJhzVoxLWYHw3LLaVBvjB1oGzcvQAbyYx/7TB UXzutEJvvX1V8MvE4Yeq18/+UEIkpZ3bMOFyaBhXkHgUSGv4y9KqOyt8F96oA++7+4s9Dv +nkD3bn/rFXUaCu8BcosSHIwacTRukXnAq6ribWleQ14eXNE4TFjH+kw35T152z4IablWk 6ylXbQNhIQLTQWI5yjKnSkjU+AFrq/dCN3RcpcJpU9wbNYXnSiStzpEnVlbeHDiKs7teIm 7d8WhkbhrxnikphYTcAGeDIN5yoMQ2bbn6GTDbppw9NG8JmoHuRCwZwhoMSNWA== 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 4jG0y30JBrz12Cd for ; Thu, 29 Oct 2026 23:37:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3ba37 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Thu, 29 Oct 2026 23:37:15 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Cc: Sumit Saxena From: Kevin Bowling Subject: git: 041be445951d - stable/15 - if_bnxt: log: fix missing lock coverage in bnxt_log_live() 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: kbowling X-Git-Repository: src X-Git-Refname: refs/heads/stable/15 X-Git-Reftype: branch X-Git-Commit: 041be445951d3062646ec448a6d8e9d30c665f30 Auto-Submitted: auto-generated Date: Thu, 29 Oct 2026 23:37:15 +0000 Message-Id: <6ae3d8ab.3ba37.5f028ba0@gitrepo.freebsd.org> The branch stable/15 has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=041be445951d3062646ec448a6d8e9d30c665f30 commit 041be445951d3062646ec448a6d8e9d30c665f30 Author: Sumit Saxena AuthorDate: 2026-08-05 04:07:40 +0000 Commit: Kevin Bowling CommitDate: 2026-10-29 23:15:19 +0000 if_bnxt: log: fix missing lock coverage in bnxt_log_live() bnxt_log_live() walked the loggers TAILQ without holding log_lock, unlike every other bnxt_log.c function, racing against concurrent bnxt_register_logger()/bnxt_unregister_logger() calls. Take log_lock around the traversal, and bail out early (with the lock dropped) if live_msgs_len has already reached max_live_buff_size, which would otherwise underflow the length passed to bnxt_log_info(). bnxt_start_logging_driver_coredump() now drops log_lock before invoking logger->log_live_op() (which calls back into bnxt_log_live()) and re-acquires it afterward, resetting live_msgs to NULL so a later bnxt_log_live() call (e.g. from a VF async event handler) can't write into a coredump buffer the caller has already freed. Reviewed by: gallatin Sponsored by: Broadcom Inc. Differential Revision: https://reviews.freebsd.org/D59086 (cherry picked from commit fc4cd2c477e6d4a2cb241c0bbac0fb91520cddcf) --- sys/dev/bnxt/bnxt_en/bnxt_log.c | 80 ++++++++++++++++++++++++++++++++++------- sys/dev/bnxt/bnxt_en/bnxt_log.h | 4 +++ 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/sys/dev/bnxt/bnxt_en/bnxt_log.c b/sys/dev/bnxt/bnxt_en/bnxt_log.c index e891dab80491..90b3408ab7ff 100644 --- a/sys/dev/bnxt/bnxt_en/bnxt_log.c +++ b/sys/dev/bnxt/bnxt_en/bnxt_log.c @@ -99,21 +99,30 @@ void bnxt_unregister_logger(struct bnxt_softc *bp, int logger_id) { struct bnxt_logger *l = NULL, *tmp; + bool found = false, defer = false; mtx_lock(&bp->log_lock); TAILQ_FOREACH_SAFE(l, &bp->loggers_list, list, tmp) { if (l->logger_id == logger_id) { TAILQ_REMOVE(&bp->loggers_list, l, list); + found = true; break; } } + if (found && l->refcnt > 0) { + /* Pinned by coredump collection; defer the free. */ + l->unregister_pending = true; + defer = true; + } mtx_unlock(&bp->log_lock); - if (!l) { + if (!found) { device_printf(bp->dev, "logger id %d not registered\n", logger_id); return; } + if (defer) + return; free(l->msgs, M_BNXT_LOG); free(l, M_BNXT_LOG); @@ -137,9 +146,11 @@ bnxt_log_info(char *buf, size_t max_len, const char *format, va_list args) else if (text[text_len] == '\0') text[text_len] = '\n'; - if (text_len > max_len) { - /* Truncate */ - text_len = max_len; + /* memcpy() below writes text_len + 1 bytes; leave room for it. */ + if (max_len == 0) + return (0); + if (text_len >= max_len) { + text_len = max_len - 1; text[text_len] = '\n'; } @@ -192,17 +203,22 @@ void bnxt_log_live(struct bnxt_softc *bp, uint16_t logger_id, const char *format, ...) { - struct bnxt_logger *logger = NULL, *tmp; + struct bnxt_logger *logger = NULL; va_list args; int len; - TAILQ_FOREACH_SAFE(logger, &bp->loggers_list, list, tmp) { + mtx_lock(&bp->log_lock); + /* log_lock stays held for this traversal, so plain TAILQ_FOREACH is safe here. */ + TAILQ_FOREACH(logger, &bp->loggers_list, list) { if (logger->logger_id == logger_id) break; } - if (!logger || !logger->live_msgs) + if (!logger || !logger->live_msgs || + logger->live_msgs_len >= logger->max_live_buff_size) { + mtx_unlock(&bp->log_lock); return; + } va_start(args, format); len = bnxt_log_info( @@ -212,6 +228,7 @@ bnxt_log_live(struct bnxt_softc *bp, uint16_t logger_id, va_end(args); logger->live_msgs_len += len; + mtx_unlock(&bp->log_lock); } static size_t @@ -269,7 +286,7 @@ bnxt_get_loggers_coredump_size(struct bnxt_softc *bp) int bnxt_start_logging_driver_coredump(struct bnxt_softc *bp, char *dest_buf) { - struct bnxt_logger *logger, *tmp; + struct bnxt_logger *logger; size_t offset = 0; uint32_t seg_id = 0; @@ -277,27 +294,64 @@ bnxt_start_logging_driver_coredump(struct bnxt_softc *bp, char *dest_buf) return (0); mtx_lock(&bp->log_lock); - TAILQ_FOREACH_SAFE(logger, &bp->loggers_list, list, tmp) { + logger = TAILQ_FIRST(&bp->loggers_list); + while (logger != NULL) { struct bnxt_coredump_segment_hdr seg_hdr; void *seg_hdr_dest = dest_buf + offset; - size_t len; + struct bnxt_logger *next; + uint16_t next_id = 0; + bool have_next; + size_t len, live_len; offset += sizeof(seg_hdr); /* First collect logs from buffer */ len = bnxt_collect_logs_buffer(logger, dest_buf + offset); offset += len; - /* Let logger to collect live messages */ + + /* Pin logger and remember successor by id; log_lock drops below. */ + logger->refcnt++; + next = TAILQ_NEXT(logger, list); + have_next = (next != NULL); + if (have_next) + next_id = next->logger_id; + + /* log_live_op() calls bnxt_log_live(), which takes log_lock itself. */ logger->live_msgs = dest_buf + offset; logger->live_msgs_len = 0; + mtx_unlock(&bp->log_lock); + logger->log_live_op(bp); - len += logger->buffer_size; - offset += logger->buffer_size; + mtx_lock(&bp->log_lock); + logger->live_msgs = NULL; + /* Actual bytes written, not buffer_size, or offset desyncs. */ + live_len = logger->live_msgs_len; + logger->live_msgs_len = 0; + + len += live_len; + offset += live_len; bnxt_fill_coredump_seg_hdr(bp, &seg_hdr, NULL, len, 0, 0, 0, 13, seg_id); memcpy(seg_hdr_dest, &seg_hdr, sizeof(seg_hdr)); seg_id++; + + logger->refcnt--; + if (logger->refcnt == 0 && logger->unregister_pending) { + free(logger->msgs, M_BNXT_LOG); + free(logger, M_BNXT_LOG); + } + + /* Re-find successor by id; stop if it was unregistered meanwhile. */ + logger = NULL; + if (have_next) { + TAILQ_FOREACH(next, &bp->loggers_list, list) { + if (next->logger_id == next_id) { + logger = next; + break; + } + } + } } mtx_unlock(&bp->log_lock); return (offset); diff --git a/sys/dev/bnxt/bnxt_en/bnxt_log.h b/sys/dev/bnxt/bnxt_en/bnxt_log.h index ad92065bf76d..bde1c2102766 100644 --- a/sys/dev/bnxt/bnxt_en/bnxt_log.h +++ b/sys/dev/bnxt/bnxt_en/bnxt_log.h @@ -47,6 +47,10 @@ struct bnxt_logger { uint32_t max_live_buff_size; uint32_t live_msgs_len; void (*log_live_op)(void *dev); + /* Pin count held while log_lock is dropped for log_live_op(). */ + u_int refcnt; + /* Set by bnxt_unregister_logger() when refcnt > 0; defers the free. */ + bool unregister_pending; }; struct bnxt_coredump_segment_hdr {