[PATCH 1/2] efivars: Check max_size only if it is non-zero.

[PATCH 1/2] efivars: Check max_size only if it is non-zero.

[Richard Weinberger]

Some (broken?) EFI implementations return always a MaximumVariableSize of 0,
check against max_size only if it is non-zero.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/firmware/efivars.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 7acafb8..8e87f8d 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -449,7 +449,8 @@ check_var_size_locked(struct efivars *efivars, u32 attributes,
 	if (status != EFI_SUCCESS)
 		return status;
 
-	if (!storage_size || size > remaining_size || size > max_size ||
+	if (!storage_size || size > remaining_size ||
+	    (max_size && size > max_size) ||
 	    (remaining_size - size) < (storage_size / 2))
 		return EFI_OUT_OF_RESOURCES;
 
-- 
1.8.1.4

[PATCH 2/2] efivars: Implement no_storage_paranoia parameter

[Richard Weinberger]

Using this parameter one can disable the storage_size/2 check if
he is really sure that the UEFI does sane gc.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/firmware/efivars.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 8e87f8d..0e1d669 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -105,8 +105,10 @@ MODULE_VERSION(EFIVARS_VERSION);
 
 static bool efivars_pstore_disable =
 	IS_ENABLED(CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE);
+static bool efivars_no_storage_paranoia;
 
 module_param_named(pstore_disable, efivars_pstore_disable, bool, 0644);
+module_param_named(no_storage_paranoia, efivars_no_storage_paranoia, bool, 0644);
 
 /*
  * The maximum size of VariableName + Data = 1024
@@ -450,7 +452,10 @@ check_var_size_locked(struct efivars *efivars, u32 attributes,
 		return status;
 
 	if (!storage_size || size > remaining_size ||
-	    (max_size && size > max_size) ||
+	    (max_size && size > max_size))
+		return EFI_OUT_OF_RESOURCES;
+
+	if (!efivars_no_storage_paranoia &&
 	    (remaining_size - size) < (storage_size / 2))
 		return EFI_OUT_OF_RESOURCES;
 
-- 
1.8.1.4

RE: [PATCH 1/2] efivars: Check max_size only if it is non-zero.

[Luck, Tony]

> Some (broken?) EFI implementations return always a MaximumVariableSize of 0,
> check against max_size only if it is non-zero.

The spec doesn't say that zero has any special meaning - so if an implementation
returns max_size == 0 but lets you set a variable to a size > 0, then I don't think
there is a need for parentheses or a "?" in this commit comment.

But if Linux silently accepts such broken EFI, then there is no feedback loop
to let EFI implementations know that they are broken.  In other areas we have
thrown out messages about firmware being broken ... perhaps:

	if (max_size == 0)
		printk_once("Broken EFI implementation is returning MaxVariableSize=0\n");

would help? After all there probably *is* a maximum size - but EFI isn't telling us what it is.

-Tony

Re: [PATCH 1/2] efivars: Check max_size only if it is non-zero.

[Richard Weinberger]

Am 04.04.2013 18:00, schrieb Luck, Tony:
>> Some (broken?) EFI implementations return always a MaximumVariableSize of 0,
>> check against max_size only if it is non-zero.
>
> The spec doesn't say that zero has any special meaning - so if an implementation
> returns max_size == 0 but lets you set a variable to a size > 0, then I don't think
> there is a need for parentheses or a "?" in this commit comment.

Thanks for the clarification.
Yesterday I've looked into the spec, but the >2000 pages hurt my brain. ;-)

> But if Linux silently accepts such broken EFI, then there is no feedback loop
> to let EFI implementations know that they are broken.  In other areas we have
> thrown out messages about firmware being broken ... perhaps:
>
> 	if (max_size == 0)
> 		printk_once("Broken EFI implementation is returning MaxVariableSize=0\n");
>
> would help? After all there probably *is* a maximum size - but EFI isn't telling us what it is.

Fair point. I'll add such a printk() to my patch and resend.

Thanks,
//richard

Re: [PATCH 1/2] efivars: Check max_size only if it is non-zero.

[Matt Fleming]

On 04/04/13 17:12, Richard Weinberger wrote:
> Fair point. I'll add such a printk() to my patch and resend.

Also take a look at FW_BUG.