linux-kernel - Re: [PATCH] efi: Use GFP_ATOMIC instead of GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160414214949.GS2829@codeblueprint.co.uk>
Date:	Thu, 14 Apr 2016 22:49:49 +0100
From:	Matt Fleming <matt@...eblueprint.co.uk>
To:	Vaishali Thakkar <vaishali.thakkar@...cle.com>
Cc:	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Julia Lawall <julia.lawall@...6.fr>,
	Saurabh Sengar <saurabh.truth@...il.com>
Subject: Re: [PATCH] efi: Use GFP_ATOMIC instead of GFP_KERNEL

On Mon, 11 Apr, at 04:23:29PM, Vaishali Thakkar wrote:
> Function dup_variable_bug is called inside the spinlock.
> This may lead to issues when kzalloc sleeps, so it is
> better to use GFP_ATOMIC in this spinlocked context.
> 
> Problem found using Coccinelle.
 
Dang it, I broke coccinelle ;)

> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@...cle.com>
> ---
>  drivers/firmware/efi/vars.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index 0ac594c..d5e2f28 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -411,7 +411,7 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid,
>  	 */
>  	efivar_wq_enabled = false;
>  
> -	str8 = kzalloc(len8, GFP_KERNEL);
> +	str8 = kzalloc(len8, GFP_ATOMIC);
>  	if (!str8)
>  		return;
>  

This was brought up by Saurabh last year,

  https://lkml.kernel.org/r/1446003747-3760-1-git-send-email-saurabh.truth@gmail.com

dup_variable_bug() is never called while holding the spinlock in
practice, and I'm guessing Coccinelle cannot understand that because
it'd need to look at program control flow, across multiple compilation
units.

If anyone wants to send a patch to clean up the EFI code so that it's
easier for coccinelle to check it, I'd be happy to review it.