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:   Tue, 3 Mar 2020 05:24:58 -0500 (EST)
From:   Vladis Dronov <vdronov@redhat.com>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     linux-efi <linux-efi@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Message-ID: <358842423.12639861.1583231098544.JavaMail.zimbra@redhat.com>
In-Reply-To: <1980156503.12639063.1583230452485.JavaMail.zimbra@redhat.com>
References: <20200303085528.27658-1-vdronov@redhat.com> <CAKv+Gu_3ZRRcoAcLTVVQe26q5x9KALmztaNQF=e=KqWaAwxtpA@mail.gmail.com> <1980156503.12639063.1583230452485.JavaMail.zimbra@redhat.com>
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: a9pXAtMcyqvelpyZViw1cEjneZdMYYgMv25R
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello, Ard, all,

> > Wouldn't it be easier to pass a var_data_size stack variable into
> > efivar_entry_get(), and only update the value in 'var' if it is <=
> > 1024?
> > 
> 
> I was thinking about this approach, but this way we still do not protect
> var from a concurrent access. For example, efivar_data_read() can race
> with itself:

Oh, indeed, this race is not possible the way you sugget with a var_data_size
stack variable. Unfortunately, AFAIU, the read/write race stays:
 
> ... efivar read functions still can race with the write function
> efivar_store_raw(). Surely, the race window is much smaller but it is there.
> I strongly believe we need to protect all data accesses here with a lock.

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