Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 4 Mar 2020 11:06:21 -0500 (EST)
From:   Vladis Dronov <vdronov@redhat.com>
To:     joeyli <jlee@suse.com>
Cc:     Ard Biesheuvel <ardb@kernel.org>, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Message-ID: <701521302.13078565.1583337981352.JavaMail.zimbra@redhat.com>
In-Reply-To: <20200304140720.GQ16878@linux-l9pv.suse>
References: <20200303085528.27658-1-vdronov@redhat.com> <20200304140720.GQ16878@linux-l9pv.suse>
Subject: Re: [PATCH] efi: fix a race and a buffer overflow while reading
 efivars via sysfs
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: fix a race and a buffer overflow while reading efivars via sysfs
Thread-Index: IH2nruIWMwx9vxI4ZbXE47kW8jiXfg==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@suse.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