From: Anisse Astier <[email protected]>
[ Upstream commit d86ff3333cb1d5f42d8898fb5fdb304e143c0237 ]
When writing EFI variables, one might get errors with no other message
on why it fails. Being able to see how much is used by EFI variables
helps analyzing such issues.
Since this is not a conventional filesystem, block size is intentionally
set to 1 instead of PAGE_SIZE.
x86 quirks of reserved size are taken into account; so that available
and free size can be different, further helping debugging space issues.
With this patch, one can see the remaining space in EFI variable storage
via efivarfs, like this:
$ df -h /sys/firmware/efi/efivars/
Filesystem Size Used Avail Use% Mounted on
efivarfs 176K 106K 66K 62% /sys/firmware/efi/efivars
Signed-off-by: Anisse Astier <[email protected]>
[ardb: - rename efi_reserved_space() to efivar_reserved_space()
- whitespace/coding style tweaks]
Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
arch/x86/platform/efi/quirks.c | 8 +++++++
drivers/firmware/efi/efi.c | 1 +
drivers/firmware/efi/vars.c | 12 +++++++++++
fs/efivarfs/super.c | 39 +++++++++++++++++++++++++++++++++-
include/linux/efi.h | 11 ++++++++++
5 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index b0b848d6933af..f0cc00032751d 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -114,6 +114,14 @@ void efi_delete_dummy_variable(void)
EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
}
+u64 efivar_reserved_space(void)
+{
+ if (efi_no_storage_paranoia)
+ return 0;
+ return EFI_MIN_RESERVE;
+}
+EXPORT_SYMBOL_GPL(efivar_reserved_space);
+
/*
* In the nonblocking case we do not attempt to perform garbage
* collection if we do not have enough free space. Rather, we do the
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 34b9e78765386..91d986a741dad 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -211,6 +211,7 @@ static int generic_ops_register(void)
generic_ops.get_variable = efi.get_variable;
generic_ops.get_next_variable = efi.get_next_variable;
generic_ops.query_variable_store = efi_query_variable_store;
+ generic_ops.query_variable_info = efi.query_variable_info;
if (efi_rt_services_supported(EFI_RT_SUPPORTED_SET_VARIABLE)) {
generic_ops.set_variable = efi.set_variable;
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index bfc5fa6aa47b6..e9dc7116daf13 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -245,3 +245,15 @@ efi_status_t efivar_set_variable(efi_char16_t *name, efi_guid_t *vendor,
return status;
}
EXPORT_SYMBOL_NS_GPL(efivar_set_variable, EFIVAR);
+
+efi_status_t efivar_query_variable_info(u32 attr,
+ u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size)
+{
+ if (!__efivars->ops->query_variable_info)
+ return EFI_UNSUPPORTED;
+ return __efivars->ops->query_variable_info(attr, storage_space,
+ remaining_space, max_variable_size);
+}
+EXPORT_SYMBOL_NS_GPL(efivar_query_variable_info, EFIVAR);
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 482d612b716bb..e028fafa04f38 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -13,6 +13,7 @@
#include <linux/ucs2_string.h>
#include <linux/slab.h>
#include <linux/magic.h>
+#include <linux/statfs.h>
#include "internal.h"
@@ -23,8 +24,44 @@ static void efivarfs_evict_inode(struct inode *inode)
clear_inode(inode);
}
+static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+ const u32 attr = EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS;
+ u64 storage_space, remaining_space, max_variable_size;
+ efi_status_t status;
+
+ status = efivar_query_variable_info(attr, &storage_space, &remaining_space,
+ &max_variable_size);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_err(status);
+
+ /*
+ * This is not a normal filesystem, so no point in pretending it has a block
+ * size; we declare f_bsize to 1, so that we can then report the exact value
+ * sent by EFI QueryVariableInfo in f_blocks and f_bfree
+ */
+ buf->f_bsize = 1;
+ buf->f_namelen = NAME_MAX;
+ buf->f_blocks = storage_space;
+ buf->f_bfree = remaining_space;
+ buf->f_type = dentry->d_sb->s_magic;
+
+ /*
+ * In f_bavail we declare the free space that the kernel will allow writing
+ * when the storage_paranoia x86 quirk is active. To use more, users
+ * should boot the kernel with efi_no_storage_paranoia.
+ */
+ if (remaining_space > efivar_reserved_space())
+ buf->f_bavail = remaining_space - efivar_reserved_space();
+ else
+ buf->f_bavail = 0;
+
+ return 0;
+}
static const struct super_operations efivarfs_ops = {
- .statfs = simple_statfs,
+ .statfs = efivarfs_statfs,
.drop_inode = generic_delete_inode,
.evict_inode = efivarfs_evict_inode,
};
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 571d1a6e1b744..120af31a5136f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1042,6 +1042,7 @@ struct efivar_operations {
efi_set_variable_t *set_variable;
efi_set_variable_t *set_variable_nonblocking;
efi_query_variable_store_t *query_variable_store;
+ efi_query_variable_info_t *query_variable_info;
};
struct efivars {
@@ -1049,6 +1050,12 @@ struct efivars {
const struct efivar_operations *ops;
};
+#ifdef CONFIG_X86
+u64 __attribute_const__ efivar_reserved_space(void);
+#else
+static inline u64 efivar_reserved_space(void) { return 0; }
+#endif
+
/*
* The maximum size of VariableName + Data = 1024
* Therefore, it's reasonable to save that much
@@ -1087,6 +1094,10 @@ efi_status_t efivar_set_variable_locked(efi_char16_t *name, efi_guid_t *vendor,
efi_status_t efivar_set_variable(efi_char16_t *name, efi_guid_t *vendor,
u32 attr, unsigned long data_size, void *data);
+efi_status_t efivar_query_variable_info(u32 attr, u64 *storage_space,
+ u64 *remaining_space,
+ u64 *max_variable_size);
+
#if IS_ENABLED(CONFIG_EFI_CAPSULE_LOADER)
extern bool efi_capsule_pending(int *reset_type);
--
2.39.2
From: Masahiro Yamada <[email protected]>
[ Upstream commit 8635e8df477bc77837886da206f4915576f88fec ]
This reverts commit cead61a6717a9873426b08d73a34a325e3546f5d.
It exported __stack_smash_handler and __guard, while they may not be
defined by anyone.
The code *declares* __stack_smash_handler and __guard. It does not
create weak symbols. If no external library is linked, they are left
undefined, but yet exported.
If a loadable module tries to access non-existing symbols, bad things
(a page fault, NULL pointer dereference, etc.) will happen. So, the
current code is wrong and dangerous.
If the code were written as follows, it would *define* them as weak
symbols so modules would be able to get access to them.
void (*__stack_smash_handler)(void *) __attribute__((weak));
EXPORT_SYMBOL(__stack_smash_handler);
long __guard __attribute__((weak));
EXPORT_SYMBOL(__guard);
In fact, modpost forbids exporting undefined symbols. It shows an error
message if it detects such a mistake.
ERROR: modpost: "..." [...] was exported without definition
Unfortunately, it is checked only when the code is built as modular.
The problem described above has been unnoticed for a long time because
arch/um/os-Linux/user_syms.c is always built-in.
With a planned change in Kbuild, exporting undefined symbols will always
result in a build error instead of a run-time error. It is a good thing,
but we need to fix the breakage in advance.
One fix is to define weak symbols as shown above. An alternative is to
export them conditionally as follows:
#ifdef CONFIG_STACKPROTECTOR
extern void __stack_smash_handler(void *);
EXPORT_SYMBOL(__stack_smash_handler);
external long __guard;
EXPORT_SYMBOL(__guard);
#endif
This is what other architectures do; EXPORT_SYMBOL(__stack_chk_guard)
is guarded by #ifdef CONFIG_STACKPROTECTOR.
However, adding the #ifdef guard is not sensible because UML cannot
enable the stack-protector in the first place! (Please note UML does
not select HAVE_STACKPROTECTOR in Kconfig.)
So, the code is already broken (and unused) in multiple ways.
Just remove.
Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
arch/um/os-Linux/user_syms.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/um/os-Linux/user_syms.c b/arch/um/os-Linux/user_syms.c
index 9b62a9d352b3a..a310ae27b479a 100644
--- a/arch/um/os-Linux/user_syms.c
+++ b/arch/um/os-Linux/user_syms.c
@@ -37,13 +37,6 @@ EXPORT_SYMBOL(vsyscall_ehdr);
EXPORT_SYMBOL(vsyscall_end);
#endif
-/* Export symbols used by GCC for the stack protector. */
-extern void __stack_smash_handler(void *) __attribute__((weak));
-EXPORT_SYMBOL(__stack_smash_handler);
-
-extern long __guard __attribute__((weak));
-EXPORT_SYMBOL(__guard);
-
#ifdef _FORTIFY_SOURCE
extern int __sprintf_chk(char *str, int flag, size_t len, const char *format);
EXPORT_SYMBOL(__sprintf_chk);
--
2.39.2
On Mon, 24 Jul 2023 at 03:31, Sasha Levin <[email protected]> wrote:
>
> From: Anisse Astier <[email protected]>
>
> [ Upstream commit d86ff3333cb1d5f42d8898fb5fdb304e143c0237 ]
>
NAK
Please don't backport this - this is obviously new functionality.
> When writing EFI variables, one might get errors with no other message
> on why it fails. Being able to see how much is used by EFI variables
> helps analyzing such issues.
>
> Since this is not a conventional filesystem, block size is intentionally
> set to 1 instead of PAGE_SIZE.
>
> x86 quirks of reserved size are taken into account; so that available
> and free size can be different, further helping debugging space issues.
>
> With this patch, one can see the remaining space in EFI variable storage
> via efivarfs, like this:
>
> $ df -h /sys/firmware/efi/efivars/
> Filesystem Size Used Avail Use% Mounted on
> efivarfs 176K 106K 66K 62% /sys/firmware/efi/efivars
>
> Signed-off-by: Anisse Astier <[email protected]>
> [ardb: - rename efi_reserved_space() to efivar_reserved_space()
> - whitespace/coding style tweaks]
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> arch/x86/platform/efi/quirks.c | 8 +++++++
> drivers/firmware/efi/efi.c | 1 +
> drivers/firmware/efi/vars.c | 12 +++++++++++
> fs/efivarfs/super.c | 39 +++++++++++++++++++++++++++++++++-
> include/linux/efi.h | 11 ++++++++++
> 5 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index b0b848d6933af..f0cc00032751d 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -114,6 +114,14 @@ void efi_delete_dummy_variable(void)
> EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
> }
>
> +u64 efivar_reserved_space(void)
> +{
> + if (efi_no_storage_paranoia)
> + return 0;
> + return EFI_MIN_RESERVE;
> +}
> +EXPORT_SYMBOL_GPL(efivar_reserved_space);
> +
> /*
> * In the nonblocking case we do not attempt to perform garbage
> * collection if we do not have enough free space. Rather, we do the
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 34b9e78765386..91d986a741dad 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -211,6 +211,7 @@ static int generic_ops_register(void)
> generic_ops.get_variable = efi.get_variable;
> generic_ops.get_next_variable = efi.get_next_variable;
> generic_ops.query_variable_store = efi_query_variable_store;
> + generic_ops.query_variable_info = efi.query_variable_info;
>
> if (efi_rt_services_supported(EFI_RT_SUPPORTED_SET_VARIABLE)) {
> generic_ops.set_variable = efi.set_variable;
> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index bfc5fa6aa47b6..e9dc7116daf13 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -245,3 +245,15 @@ efi_status_t efivar_set_variable(efi_char16_t *name, efi_guid_t *vendor,
> return status;
> }
> EXPORT_SYMBOL_NS_GPL(efivar_set_variable, EFIVAR);
> +
> +efi_status_t efivar_query_variable_info(u32 attr,
> + u64 *storage_space,
> + u64 *remaining_space,
> + u64 *max_variable_size)
> +{
> + if (!__efivars->ops->query_variable_info)
> + return EFI_UNSUPPORTED;
> + return __efivars->ops->query_variable_info(attr, storage_space,
> + remaining_space, max_variable_size);
> +}
> +EXPORT_SYMBOL_NS_GPL(efivar_query_variable_info, EFIVAR);
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index 482d612b716bb..e028fafa04f38 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -13,6 +13,7 @@
> #include <linux/ucs2_string.h>
> #include <linux/slab.h>
> #include <linux/magic.h>
> +#include <linux/statfs.h>
>
> #include "internal.h"
>
> @@ -23,8 +24,44 @@ static void efivarfs_evict_inode(struct inode *inode)
> clear_inode(inode);
> }
>
> +static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
> +{
> + const u32 attr = EFI_VARIABLE_NON_VOLATILE |
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS;
> + u64 storage_space, remaining_space, max_variable_size;
> + efi_status_t status;
> +
> + status = efivar_query_variable_info(attr, &storage_space, &remaining_space,
> + &max_variable_size);
> + if (status != EFI_SUCCESS)
> + return efi_status_to_err(status);
> +
> + /*
> + * This is not a normal filesystem, so no point in pretending it has a block
> + * size; we declare f_bsize to 1, so that we can then report the exact value
> + * sent by EFI QueryVariableInfo in f_blocks and f_bfree
> + */
> + buf->f_bsize = 1;
> + buf->f_namelen = NAME_MAX;
> + buf->f_blocks = storage_space;
> + buf->f_bfree = remaining_space;
> + buf->f_type = dentry->d_sb->s_magic;
> +
> + /*
> + * In f_bavail we declare the free space that the kernel will allow writing
> + * when the storage_paranoia x86 quirk is active. To use more, users
> + * should boot the kernel with efi_no_storage_paranoia.
> + */
> + if (remaining_space > efivar_reserved_space())
> + buf->f_bavail = remaining_space - efivar_reserved_space();
> + else
> + buf->f_bavail = 0;
> +
> + return 0;
> +}
> static const struct super_operations efivarfs_ops = {
> - .statfs = simple_statfs,
> + .statfs = efivarfs_statfs,
> .drop_inode = generic_delete_inode,
> .evict_inode = efivarfs_evict_inode,
> };
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 571d1a6e1b744..120af31a5136f 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1042,6 +1042,7 @@ struct efivar_operations {
> efi_set_variable_t *set_variable;
> efi_set_variable_t *set_variable_nonblocking;
> efi_query_variable_store_t *query_variable_store;
> + efi_query_variable_info_t *query_variable_info;
> };
>
> struct efivars {
> @@ -1049,6 +1050,12 @@ struct efivars {
> const struct efivar_operations *ops;
> };
>
> +#ifdef CONFIG_X86
> +u64 __attribute_const__ efivar_reserved_space(void);
> +#else
> +static inline u64 efivar_reserved_space(void) { return 0; }
> +#endif
> +
> /*
> * The maximum size of VariableName + Data = 1024
> * Therefore, it's reasonable to save that much
> @@ -1087,6 +1094,10 @@ efi_status_t efivar_set_variable_locked(efi_char16_t *name, efi_guid_t *vendor,
> efi_status_t efivar_set_variable(efi_char16_t *name, efi_guid_t *vendor,
> u32 attr, unsigned long data_size, void *data);
>
> +efi_status_t efivar_query_variable_info(u32 attr, u64 *storage_space,
> + u64 *remaining_space,
> + u64 *max_variable_size);
> +
> #if IS_ENABLED(CONFIG_EFI_CAPSULE_LOADER)
> extern bool efi_capsule_pending(int *reset_type);
>
> --
> 2.39.2
>