[PATCH net] netconsole: harden the enablement through configfs.

[PATCH net] netconsole: harden the enablement through configfs.

[Breno Leitao]

There is a possible race when enabling/disabling the target through
configfs with netdev events that can call a null pointer dereference.

My robot was able to come up with a reproducer by adding a bunch of
msleeps() around.

The race is basically this one:

  CPU 0 (enabled_store)             CPU 1 (netconsole_netdev_event)
  current_enabled = true
                                    nt->state = STATE_DEACTIVATED
                                    do_netpoll_cleanup()
                                      np->dev = NULL
  nt->state = STATE_DISABLED
  list_move(&nt->list, &target_cleanup_list)
  do_netpoll_cleanup()
    __netpoll_cleanup(): np->dev->npinfo

And kasan reports:

  KASAN: null-ptr-deref in range [0x0000000000000108-0x000000000000010f]
  RIP: 0010:__netpoll_cleanup
  Call Trace:
   do_netpoll_cleanup
   netconsole_process_cleanups_core
   enabled_store
   configfs_write_iter

The disable path used to call netpoll_cleanup(), which skips a netpoll
without a device, until it started deferring the release to
target_cleanup_list.

Re-check the state under target_list_lock, which the notifier holds when
it stops a target, and queue the target only if it is still enabled.
Mark it disabled either way, as before.

Signed-off-by: Breno Leitao <leitao@debian.org>
Fixes: 97714695ef90 ("net: netconsole: Defer netpoll cleanup to avoid lock release during list traversal")
---
Given this is not super critical or hardly faced in production, I am
sending this to net-next, even with a 'Fixes:' tag.
---
 drivers/net/netconsole.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index a9f9477da1ec77..4b546194f7179c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -1143,11 +1143,10 @@ static ssize_t enabled_store(struct config_item *item,
 		 */
 		mutex_lock(&target_cleanup_list_lock);
 		spin_lock_irqsave(&target_list_lock, flags);
+		/* An interface event may have stopped and released it */
+		if (nt->state == STATE_ENABLED)
+			list_move(&nt->list, &target_cleanup_list);
 		nt->state = STATE_DISABLED;
-		/* Remove the target from the list, while holding
-		 * target_list_lock
-		 */
-		list_move(&nt->list, &target_cleanup_list);
 		spin_unlock_irqrestore(&target_list_lock, flags);
 		mutex_unlock(&target_cleanup_list_lock);
 		netconsole_unregister_console(nt);

---
base-commit: c88fd6989aea9237a60a4859139a265bfb7ddc0b
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git main
change-id: 20261112-netcons_double_cleanup-43b00118c1c9

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH net] netconsole: harden the enablement through configfs.

[Gustavo Luiz Duarte]

On Thu, Nov 12, 2026 at 2:05 PM Breno Leitao <leitao@debian.org> wrote:
>
> There is a possible race when enabling/disabling the target through
> configfs with netdev events that can call a null pointer dereference.
>
> My robot was able to come up with a reproducer by adding a bunch of
> msleeps() around.
>
> The race is basically this one:
>
>   CPU 0 (enabled_store)             CPU 1 (netconsole_netdev_event)
>   current_enabled = true
>                                     nt->state = STATE_DEACTIVATED
>                                     do_netpoll_cleanup()
>                                       np->dev = NULL
>   nt->state = STATE_DISABLED
>   list_move(&nt->list, &target_cleanup_list)
>   do_netpoll_cleanup()
>     __netpoll_cleanup(): np->dev->npinfo
>
> And kasan reports:
>
>   KASAN: null-ptr-deref in range [0x0000000000000108-0x000000000000010f]
>   RIP: 0010:__netpoll_cleanup
>   Call Trace:
>    do_netpoll_cleanup
>    netconsole_process_cleanups_core
>    enabled_store
>    configfs_write_iter
>
> The disable path used to call netpoll_cleanup(), which skips a netpoll
> without a device, until it started deferring the release to
> target_cleanup_list.
>
> Re-check the state under target_list_lock, which the notifier holds when
> it stops a target, and queue the target only if it is still enabled.
> Mark it disabled either way, as before.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>

Good catch!
Reviewed-by: Gustavo Luiz Duarte <gustavold@gmail.com>

Re: [PATCH net] netconsole: harden the enablement through configfs.

[Breno Leitao]

On Thu, Nov 12, 2026 at 06:04:58AM -0800, Breno Leitao wrote:
> ---
> Given this is not super critical or hardly faced in production, I am
> sending this to net-next, even with a 'Fixes:' tag.

My tag ended up being 'net' instead of 'net-next', sorry. This patch was
made on net-next, and that is where we should have apply, ignoring the
`net` tag.

--breno