From nobody Fri Oct 09 10:12:58 2026 X-Original-To: dev-commits-src-main@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 4j1N3H3b1Gz6vRZm for ; Fri, 09 Oct 2026 10:12:59 +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 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4j1N3H1J39z4Y3P for ; Fri, 09 Oct 2026 10:12:59 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791540779; 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=eULoXhEy3slpvzrw5LXfTWASpg5uyRATdtjeB7rFmw4=; b=hxc1Nrh8kaemUtF/FznjcIXOiKv4LRUO/b1+et5+9eOmfYAEa9AYd9c8gFjdfZIGDjTBOu bX5t4AaAKomjHs6B5aoEjn68hZotMfDEj/tRSr6ih/2xrGm2Bo3uS+3n5D0zmQ0ZA1f31s 9AqLLt2DtErqtFbCeZQnavJbV9WS8Y0fWxKvuLYTqj4NRJUJU+u4q5h652cV7cFriF9HBd 0PZ6DK1YN7gSOQ6aIskH+Px5nZvZd2/C9gJ2b3wAwhlfN2v9iUHONAiFvd4wnWeHDIZRns 99F+pWUgb/S5CZ6oJUSGZDaJABEaAGSJlVaKC4YbcXF69A+UBW8PMl8un0wy7g== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1791540779; b=Nkz0KpxX4WkunG7nJG7rqYTENiu6BhER5yi+HJQ2CIzZPYEqn/LR/dyC4UCCTi5p25qy0V tGHMNfa0YEm43BaHcVO1SltYlnBOSpoUzVs2ibEVyuxkXwZYSxnExuEmhzTQAn3bLp4qBQ ZoPBG7DYlKkqFji5XuyrVqnzlp7cDsCZB8Q6aZeDEZ/Fzavly1S4psH5sAsuhfN5HIDVVr AqPVuAGkOf5bcIZWH6qVSXWYXtryw0tdAccqLDGFSpL/PqvsFVLwZmMomtxxo+d5j5eZKQ 9SLbe8NFRdKRGj1dT2SJKwciAYdeUXMCJ8/SU/fswm1qlfggG4xd4vIhYwPLqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791540779; 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=eULoXhEy3slpvzrw5LXfTWASpg5uyRATdtjeB7rFmw4=; b=EyIjH1xwgiVscK1PfJzFYRanGvCuddk8qI2dw0Plj7R03m/a0S53e/Cua8VGZOc9+Sf1hr I3HUiceRgxPRnDCJfYKRJaD9OPOH7xDC7Um8Xa75ilKdgoBObdVUeBcd8yDKBsus4zpCbS jD+NCnj37xkA/OppUQKBX+Q8wCJnYJz2Li/Kbv5xasATNfdrPa5r1aORb1/da4XsPbWk4D Xi9sVDEXMYKy760M4JmmC0yKk+aJV2CWbYVPSJ9LOT1l5tYNMI18tLItr3cpoF/1BR+HBL gM92r2ThV9SWnuKBQh85z4WC/PAmyTaSY9hdLXNQqWvTW+Bv6DFGHab83m/NdA== 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 4j1N3G6cNQz1L1S for ; Fri, 09 Oct 2026 10:12:58 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 43a8b by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 09 Oct 2026 10:12:58 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Sumit Saxena From: Sumit Saxena Subject: git: fc4cd2c477e6 - main - if_bnxt: log: fix missing lock coverage in bnxt_log_live() List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org List-Id: 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: ssaxena X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: fc4cd2c477e6d4a2cb241c0bbac0fb91520cddcf Auto-Submitted: auto-generated Date: Fri, 09 Oct 2026 10:12:58 +0000 Message-Id: <6ac8be2a.43a8b.2d463500@gitrepo.freebsd.org> The branch main has been updated by ssaxena: URL: https://cgit.FreeBSD.org/src/commit/?id=fc4cd2c477e6d4a2cb241c0bbac0fb91520cddcf commit fc4cd2c477e6d4a2cb241c0bbac0fb91520cddcf Author: Sumit Saxena AuthorDate: 2026-08-05 04:07:40 +0000 Commit: Sumit Saxena CommitDate: 2026-10-09 08:24:45 +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 MFC after: 2 weeks Sponsored by: Broadcom Inc. Differential Revision: https://reviews.freebsd.org/D59086 --- 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 {