Date: Mon, 17 Jul 2017 09:38:54 -0700 From: John Baldwin <jhb@freebsd.org> To: freebsd-acpi@freebsd.org Cc: Jia-Ju Bai <baijiaju1990@163.com>, freebsd-drivers@freebsd.org Subject: Re: [Bug 220096][PATCH] acpi_thermal: Fix a possible sleep-under-mutex bug in acpi_tz_thread Message-ID: <6854694.QsTIBa8hWt@ralph.baldwin.cx> In-Reply-To: <20170618095245.40693-1-baijiaju1990@163.com> References: <20170618095245.40693-1-baijiaju1990@163.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6854694.QsTIBa8hWt>