mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chengfeng Ye <dg573847474@gmail.com>
To: kernel-team@meta.com, andrew+netdev@lunn.ch, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, jacob.e.keller@intel.com,
	lee@trager.us, horms@kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Chengfeng Ye <dg573847474@gmail.com>
Subject: [PATCH] fbnic: close fw_log race between users and teardown
Date: Mon,  9 Feb 2026 16:38:54 +0000	[thread overview]
Message-ID: <20260209163854.521415-1-dg573847474@gmail.com> (raw)

Fixes a theoretical race on fw_log between the teardown path and fw_log
show/write functions.

The fw_log null check was performed outside the lock in both
fbnic_dbg_fw_log_show() and fbnic_fw_log_write() (called from debugfs
paths or mailbox threaded IRQ handler). Concurrent teardown in
fbnic_fw_log_free() could clear and free the log buffer after the check
because there is no proper synchronization, leading to list traversal or
buffer access on freed memory.

fbnic_fw_log_write() can be reached from the mailbox handler
fbnic_fw_msix_intr(), but fbnic_fw_log_free() runs before IRQ/MBX
teardown. fbnic_dbg_fw_log_show() runs via debugfs and seems to be
not synchronized with removal either.

Possible Interleaving scenario:
  CPU0: fbnic_dbg_fw_log_show() or fbnic_fw_log_write()
          if (fbnic_fw_log_ready())   // true
          ... preempt ...
  CPU1: fbnic_fw_log_free()
          vfree(log->data_start);
          log->data_start = NULL;
  CPU0: continues, walks log->entries or writes to log->data_start

Move readiness checks under the fw_log spinlock, and clear the log state
under the same lock in fbnic_fw_log_free() before freeing the buffer.
This makes readers/writers mutually exclusive with teardown.

Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
---
 .../net/ethernet/meta/fbnic/fbnic_debugfs.c   |  8 ++++---
 .../net/ethernet/meta/fbnic/fbnic_fw_log.c    | 21 +++++++++++++------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_debugfs.c b/drivers/net/ethernet/meta/fbnic/fbnic_debugfs.c
index b7238dd967fe..4171dde590fc 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_debugfs.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_debugfs.c
@@ -176,11 +176,13 @@ static int fbnic_dbg_fw_log_show(struct seq_file *s, void *v)
 	struct fbnic_fw_log_entry *entry;
 	unsigned long flags;
 
-	if (!fbnic_fw_log_ready(fbd))
-		return -ENXIO;
-
 	spin_lock_irqsave(&fbd->fw_log.lock, flags);
 
+	if (!fbnic_fw_log_ready(fbd)) {
+		spin_unlock_irqrestore(&fbd->fw_log.lock, flags);
+		return -ENXIO;
+	}
+
 	list_for_each_entry_reverse(entry, &fbd->fw_log.entries, list) {
 		seq_printf(s, FBNIC_FW_LOG_FMT, entry->index,
 			   (entry->timestamp / (MSEC_PER_SEC * 60 * 60 * 24)),
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw_log.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw_log.c
index 85a883dba385..d371932435e5 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw_log.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw_log.c
@@ -59,16 +59,24 @@ int fbnic_fw_log_init(struct fbnic_dev *fbd)
 void fbnic_fw_log_free(struct fbnic_dev *fbd)
 {
 	struct fbnic_fw_log *log = &fbd->fw_log;
-
-	if (!fbnic_fw_log_ready(fbd))
-		return;
+	unsigned long flags;
+	void *data;
 
 	fbnic_fw_log_disable(fbd);
+
+	spin_lock_irqsave(&log->lock, flags);
+	if (!fbnic_fw_log_ready(fbd)) {
+		spin_unlock_irqrestore(&log->lock, flags);
+		return;
+	}
+	data = log->data_start;
 	INIT_LIST_HEAD(&log->entries);
 	log->size = 0;
-	vfree(log->data_start);
 	log->data_start = NULL;
 	log->data_end = NULL;
+	spin_unlock_irqrestore(&log->lock, flags);
+
+	vfree(data);
 }
 
 int fbnic_fw_log_write(struct fbnic_dev *fbd, u64 index, u32 timestamp,
@@ -80,13 +88,14 @@ int fbnic_fw_log_write(struct fbnic_dev *fbd, u64 index, u32 timestamp,
 	unsigned long flags;
 	void *entry_end;
 
+	spin_lock_irqsave(&log->lock, flags);
+
 	if (!fbnic_fw_log_ready(fbd)) {
+		spin_unlock_irqrestore(&log->lock, flags);
 		dev_err(fbd->dev, "Firmware sent log entry without being requested!\n");
 		return -ENOSPC;
 	}
 
-	spin_lock_irqsave(&log->lock, flags);
-
 	if (list_empty(&log->entries)) {
 		entry = log->data_start;
 	} else {
-- 
2.25.1


             reply	other threads:[~2026-02-09 16:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09 16:38 Chengfeng Ye [this message]
2026-02-10  2:40 ` Lee Trager
2026-02-10 12:30   ` Chengfeng Ye
2026-02-10 17:24 ` Alexander H Duyck
2026-02-11  6:27   ` Chengfeng Ye

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260209163854.521415-1-dg573847474@gmail.com \
    --to=dg573847474@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jacob.e.keller@intel.com \
    --cc=kernel-team@meta.com \
    --cc=lee@trager.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®