linux-kernel - Re: [PATCH] efi: fix a race and a buffer overflow while reading efivars via sysfs

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]

Date:   Wed, 4 Mar 2020 11:06:21 -0500 (EST)
From:   Vladis Dronov <vdronov@...hat.com>
To:     joeyli <jlee@...e.com>
Cc:     Ard Biesheuvel <ardb@...nel.org>, linux-efi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] efi: fix a race and a buffer overflow while reading
 efivars via sysfs

Hello, Joey, all,

Let me address both of your emails here.

> Looks that kernel uses EFI protocol to query variable everytime, then
> why should kernel keeps a copy of variable data size, data and attributes in
> memory? It makes sense to keep VariableName and VendorGuid, but why data?
> 
> The efi_variable can be used to interactive with userland. But we do not
> need to keep a data copy in efi_variable with efivar_entry. e.g. The
> efivarfs_file_read() allocates a buffer for reading variable instead
> of using efi_variable->Data.

Indeed, as far as I understand the code, we keep var's data in a memory. I
cannot tell why such was done when this code was written. Given that this
code is considered "old" and may even be obsoleted, I wouldn't like to start
a deep rewrite, and only focus on fixing bugs, like the one mentioned.

> I have reviewed and tested this patch. It's good to me if we still want
> to use efi_variable structure as the return buffer of UEFI get/set_variable
> protocols.
> 
> Please feel free to add:
> Reviewed-by: Joey Lee <jlee@...e.com>

Thank you for your time on the review! Much appreciated. Still, I've just
sent out a v2 patch (with you in To:) which is indeed simpler and implements
and idea Ard suggested, and fixes only the exact bug mentioned. I'm not sure
which one will be accepted (if any), could you please, to look at v2 also?

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer