From owner-freebsd-drivers@freebsd.org Sat Jul 1 00:13:20 2017 Return-Path: Delivered-To: freebsd-drivers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4805AD9E5F7; Sat, 1 Jul 2017 00:13:20 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (turbocat.net [IPv6:2a01:4f8:c17:6c4b::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 13E2083C03; Sat, 1 Jul 2017 00:13:19 +0000 (UTC) (envelope-from hps@selasky.org) Received: from hps2016.home.selasky.org (unknown [62.141.129.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id 477462602CC; Sat, 1 Jul 2017 02:13:16 +0200 (CEST) Subject: Re: [Bug 220096][PATCH] acpi_thermal: Fix a possible sleep-under-mutex bug in acpi_tz_thread To: Jia-Ju Bai Cc: freebsd-acpi@freebsd.org, freebsd-drivers@freebsd.org References: <20170618095245.40693-1-baijiaju1990@163.com> From: Hans Petter Selasky Message-ID: Date: Sat, 1 Jul 2017 02:11:07 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170618095245.40693-1-baijiaju1990@163.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Jul 2017 00:13:20 -0000 On 06/18/17 11:52, 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 > --- > 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]); > } > If you change to M_NOWAIT you need to add code to check if sc == NULL. --HPS