From owner-freebsd-acpi@freebsd.org  Sat Jul  1 00:13:20 2017
Return-Path: <owner-freebsd-acpi@freebsd.org>
Delivered-To: freebsd-acpi@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 <baijiaju1990@163.com>
Cc: freebsd-acpi@freebsd.org, freebsd-drivers@freebsd.org
References: <20170618095245.40693-1-baijiaju1990@163.com>
From: Hans Petter Selasky <hps@selasky.org>
Message-ID: <c3fabef5-1d08-fb57-9a09-9f7da7b664ce@selasky.org>
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-acpi@freebsd.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: ACPI and power management development <freebsd-acpi.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/freebsd-acpi>,
 <mailto:freebsd-acpi-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-acpi/>
List-Post: <mailto:freebsd-acpi@freebsd.org>
List-Help: <mailto:freebsd-acpi-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/freebsd-acpi>,
 <mailto:freebsd-acpi-request@freebsd.org?subject=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 <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]);
>   	}
> 

If you change to M_NOWAIT you need to add code to check if sc == NULL.

--HPS