[Bug 220096][PATCH] acpi_thermal: Fix a possible sleep-under-mutex bug in acpi_tz_thread

[John Baldwin]

On Sunday, June 18, 2017 05:52:45 PM Jia-Ju Bai wrote:
> The driver may sleep under a mutex, and the code path is:
> acpi_tz_thread [line 992: acquire the mutex]
> acpi_tz_thread [line 993]
> acpi_tz_thread [line 1003]
> acpi_tz_thread [line 1004] (msleep is excuted)
> acpi_tz_thread [line 1008]
> acpi_tz_thread [line 970]
> acpi_tz_thread [line 971]
> acpi_tz_thread [line 975]
>    malloc(M_WAITOK) [line 976]
> 
> The possible fix of this bug is to replace "M_WAITOK" in malloc with 
> "M_NOWAIT".
> 
> This bug is found by a static analysis tool written by myself, and it is 
> checked by my review of the FreeBSD code.
> 
> Signed-off-by: Jia-Ju Bai <baijiaju1990 at 163.com>
> ---
>  sys/dev/acpica/acpi_thermal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sys/dev/acpica/acpi_thermal.c b/sys/dev/acpica/acpi_thermal.c
> index b2b2a13aa88..fb9f44b5711 100644
> --- a/sys/dev/acpica/acpi_thermal.c
> +++ b/sys/dev/acpica/acpi_thermal.c
> @@ -974,7 +974,7 @@ acpi_tz_thread(void *arg)
>  	    }
>  	    devclass_get_devices(acpi_tz_devclass, &devs, &devcount);
>  	    sc = malloc(sizeof(struct acpi_tz_softc *) * devcount, M_TEMP,
> -			M_WAITOK | M_ZERO);
> +			M_NOWAIT | M_ZERO);
>  	    for (i = 0; i < devcount; i++)
>  		sc[i] = device_get_softc(devs[i]);
>  	}

As noted in the followup to the PR, the lock is never held when malloc is
called because msleep() uses PDROP.  The malloc is safe to stay as M_WAITOK.

-- 
John Baldwin