On Tue, 16 Feb, at 11:09:43AM, Peter Jones wrote:
> "rm -rf" is bricking some peoples' laptops because of variables being
> used to store non-reinitializable firmware driver data that's required
> to POST the hardware.
>
> These are 100% bugs, and they need to be fixed, but in the mean time it
> shouldn't be easy to *accidentally* brick machines.
>
> We have to have delete working, and picking which variables do and don't
> work for deletion is quite intractable, so instead make everything
> immutable by default (except for a whitelist), and make tools that
> aren't quite so broad-spectrum unset the immutable flag.
>
> Signed-off-by: Peter Jones <pjones@xxxxxxxxxx>
> ---
> Documentation/filesystems/efivarfs.txt | 7 ++
> drivers/firmware/efi/vars.c | 115 ++++++++++++++++++++-----
> fs/efivarfs/file.c | 70 +++++++++++++++
> fs/efivarfs/inode.c | 30 ++++---
> fs/efivarfs/internal.h | 3 +-
> fs/efivarfs/super.c | 9 +-
> include/linux/efi.h | 4 +-
> tools/testing/selftests/efivarfs/efivarfs.sh | 19 +++-
> tools/testing/selftests/efivarfs/open-unlink.c | 72 +++++++++++++++-
> 9 files changed, 287 insertions(+), 42 deletions(-)
[...]
> @@ -193,17 +195,59 @@ static const struct variable_validate variable_validate[] = {
> { EFI_GLOBAL_VARIABLE_GUID, "ErrOut", validate_device_path },
> { EFI_GLOBAL_VARIABLE_GUID, "ErrOutDev", validate_device_path },
> { EFI_GLOBAL_VARIABLE_GUID, "Lang", validate_ascii_string },
> + { EFI_GLOBAL_VARIABLE_GUID, "OsIndications", NULL },
> { EFI_GLOBAL_VARIABLE_GUID, "PlatformLang", validate_ascii_string },
> { EFI_GLOBAL_VARIABLE_GUID, "Timeout", validate_uint16 },
> + { LINUX_EFI_CRASH_GUID, "*", NULL },
> { NULL_GUID, "", NULL },
> };
>
You don't need to fold in patches that can be applied verbatim from
upstream. It makes validation of backports harder and the git history
bizarre.
> +static bool
> +variable_matches(const efi_char16_t *ucs2_name, const char *var_name,
> + size_t len, const char *match_name, int *match)
> +{
> + for (*match = 0; ; (*match)++) {
> + char c = match_name[*match];
> + char u = var_name[*match];
> +
> + /* Wildcard in the matching name means we've matched */
> + if (c == '*')
> + return true;
> +
> + /* All special variables are plain ascii unless they were
> + * wildcards. */
> + if (ucs2_name[*match] > 127)
> + return false;
> +
This caught my eye. You've inverted the check from here,
[...]
> - /* All special variables are plain ascii */
> - if (u > 127)
> - return true;
Was that intentional?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
