NetBSD Problem Report #57322

From www@netbsd.org  Tue Apr  4 11:13:51 2023
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E62AD1A9239
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  4 Apr 2023 11:13:50 +0000 (UTC)
Message-Id: <20230404111349.3514B1A923A@mollari.NetBSD.org>
Date: Tue,  4 Apr 2023 11:13:49 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: hdafg(4) hotplug switch detection races with suspend/resume and detach
X-Send-Pr-Version: www-1.0

>Number:         57322
>Category:       kern
>Synopsis:       hdafg(4) hotplug switch detection races with suspend/resume and detach
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 04 11:15:01 +0000 2023
>Last-Modified:  Tue Jul 18 13:40:02 +0000 2023
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, 8, ...
>Organization:
The NetBSD Founda<acpi0: entering state S3
>Environment:
on target to develop a fever of 1.5C or more
>Description:
hdafg_detach and hdafg_suspend use callout_halt to prevent any further calls to hdafg_hp_switch_handler from running, and to wait for any pending calls to complete.

However, if hdafg_hp_switch_handler is already running concurrently, it will automatically reschedule itself.  callout_halt does not prevent this.

If this happens, suspend/resume or detach might fail or crash.
>How-To-Repeat:
- code inspection
- probably suspend/resume on machine with hdaudio
>Fix:
1. Create a new lock, say sc_jack_lock, and flag, say sc_jack_detect.
2. Teach hdafg_suspend and hdafg_detach to set sc_jack_detect = true under sc_jack_lock before calling callout_halt.
3. Teach hdafg_hp_switch_handler to reschedule itself only conditionally on sc_jack_detect, under the lock.

/* hdafg_suspend/detach */
mutex_enter(&sc->sc_jack_lock);
sc->sc_jack_detect = false;
mutex_exit(&sc->sc_jack_lock);
callout_halt(&sc->sc_jack_callout, NULL);

/* hdafg_hp_switch_handler */
muteX_enter(&sc->sc_jack_lock);
if (sc->sc_jack_detect) {
        callout_schedule(&sc->sc_jack_callout, ...);
mutex_exit(&sc->sc_jack_lock);

(Maybe hdafg_hp_switch_handler should also run in thread context instead of softint context, with a kpause loop and a similar sc_jack_detect variable to suspend it.)

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57322 CVS commit: src/sys/dev/hdaudio
Date: Tue, 18 Jul 2023 13:35:58 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Jul 18 13:35:57 UTC 2023

 Modified Files:
 	src/sys/dev/hdaudio: hdafg.c

 Log Message:
 hdafg(4): Do hotplug detection in kthread, not callout.

 This can sometimes take a while (~1ms), and the logic to suspend the
 callout on device suspend/resume was racy (PR kern/57322).

 XXX pullup-8
 XXX pullup-9
 XXX pullup-10


 To generate a diff of this commit:
 cvs rdiff -u -r1.29 -r1.30 src/sys/dev/hdaudio/hdafg.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search