linux-kernel - Re: [PATCH] fbnic: close fw_log race between users and teardown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2c0395d5323770a32d53c6b874a735b888e4d4f1.camel@gmail.com>
Date: Tue, 10 Feb 2026 09:24:41 -0800
From: Alexander H Duyck <alexander.duyck@...il.com>
To: Chengfeng Ye <dg573847474@...il.com>, kernel-team@...a.com, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, 	jacob.e.keller@...el.com, lee@...ger.us,
 horms@...nel.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fbnic: close fw_log race between users and teardown

On Mon, 2026-02-09 at 16:38 +0000, Chengfeng Ye wrote:
> 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@...il.com>

You conflated two cases. One that can happen and one that can't.

>From what I can tell the debugfs issue cannot happen as the debugfs
files are removed before the log is freed. If the debugfs files can
still be accessed after they are removed we have bigger issues.

The log is initialized after the mailbox is enabled, and freed before
it is disabled so the write can currently race with the free call. That
said the current fix is pretty messy in terms of resolving it.

A better fix for this might be to pull out fbnic_fw_log_enable/disable
from the fw_log_init/free and leave them in the places currently
occupied by the fw_log_init/free calls. With that we could place the
log_init before the request_mbx call, add fw_log_free as an exception
case before "free_irqs", and relocate fw_log_free to the line after the
fw_free_mbx calls as it is a resource that should be put in place
before the mailbox is enabled, and not cleared until after the mailbox
and debugfs file has been disabled.