linux-kernel - Re: [PATCH v3] efivarfs: Request at most 512 bytes for variable names

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 for Android: free password hash cracker in your pocket

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

Message-ID: <8caa916b-5b40-446a-9a80-68a4cf0fc75f@gmx.de>
Date: Tue, 30 Jan 2024 17:00:16 +0100
From: Tim Schumacher <timschumi@....de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, jk@...abs.org, mjg59@...f.ucam.org,
 pjones@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] efivarfs: Request at most 512 bytes for variable names

On 26.01.24 19:02, Tim Schumacher wrote:
> On 26.01.24 17:35, Ard Biesheuvel wrote:
>> On Fri, 26 Jan 2024 at 17:25, Tim Schumacher <timschumi@....de> wrote:
>>
>>> One thing that I just recently noticed is that properly processing
>>> variables above 512 bytes in size is currently meaningless anyways,
>>> since the VFS layer only allows file name sizes of up to 255 bytes,
>>> and 512 bytes of UCS2 will end up being at least 256 bytes of
>>> UTF-8.
>>>
>>
>> Interesting. Let's add this to the commit log - it makes the case much
>> stronger, given that it proves that it is impossible for anyone to be
>> relying on the current maximum being over 512 bytes.
>
> It makes the case much stronger for why one wouldn't be able to _create_
> variables of that length from Linux userspace, creating dentries internally
> seems to have different restrictions (or at least their name size seems
> unlimited to me). Therefore, anything external could have still created
> such variables, and such a variable will also affect any variable that
> follows, not just itself. They don't have to be processed properly, but
> they still need to be processed (and they currently aren't processed at all).
>

I was able to experimentally confirm that creating dentries internally is
_not_ restricted by the value of NAME_MAX. The test setup was as follows:

- Build and boot a kernel with NAME_MAX bumped to an artificially high
   value (e.g. 1024). This is supposed to simulate an external user.
- Create an UEFI variable with a name of length 254 (ends up at length 291
   with the appended GUID, which is above the normal NAME_MAX limit).
- Create a "sentinel" UEFI variable with a non-critical name size (e.g. 32)
   to determine whether iteration has been stopped early during the next boot.
- Reboot into the same kernel but with an unmodified NAME_MAX limit (i.e. 255).
- Observe that not only the sentinel variable shows up (i.e. iteration
   hasn't stopped early), but that even the variable with a file name length of
   291 shows up and continues to be readable and writable from userspace.

Notably (and unexpectedly), only the _creation_ of efivarfs files with length
larger than NAME_MAX (from inside userspace) seems to abide by the NAME_MAX
limit, and ends up bailing out with "File name too long" / ENAMETOOLONG.
Therefore, please disregard my earlier statement about "processing such
entries properly is meaningless" that I put into the patch-accompanying message.
I assumed it would be enforced across all/most common file operations instead
of just when creating files.