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

[Breno Leitao <leitao@debian.org>]

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>