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

[Ard Biesheuvel]

On Tue, 30 Jan 2024 at 17:00, Tim Schumacher <timschumi@xxxxxx> wrote:
>
> 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@xxxxxx> 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.
>

Thanks for digging into this. I still think the change is reasonable:
Linux does not permit creating EFI variables that have names longer
than 512 bytes, and I have never seen any such names from any of the
firmware implementations that I have dealt with.

I have queued this up now. Thanks.