2023-03-01 16:32:25

by Sasha Levin

Subject: [PATCH AUTOSEL 4.19 2/3] efi: efivars: prevent double registration

From: Johan Hovold <[email protected]>

[ Upstream commit 0217a40d7ba6e71d7f3422fbe89b436e8ee7ece7 ]

Add the missing sanity check to efivars_register() so that it is no
longer possible to override an already registered set of efivar ops
(without first deregistering them).

This can help debug initialisation ordering issues where drivers have so
far unknowingly been relying on overriding the generic ops.

Signed-off-by: Johan Hovold <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/firmware/efi/vars.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index e619ced030d52..462e88b9d2ba4 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -1195,19 +1195,28 @@ int efivars_register(struct efivars *efivars,
const struct efivar_operations *ops,
struct kobject *kobject)
{
+ int rv;
+
if (down_interruptible(&efivars_lock))
return -EINTR;

+ if (__efivars) {
+ pr_warn("efivars already registered\n");
+ rv = -EBUSY;
+ goto out;
+ }
+
efivars->ops = ops;
efivars->kobject = kobject;

__efivars = efivars;

pr_info("Registered efivars operations\n");
-
+ rv = 0;
+out:
up(&efivars_lock);

- return 0;
+ return rv;
}
EXPORT_SYMBOL_GPL(efivars_register);

--
2.39.2

2023-03-01 16:34:30

by Ard Biesheuvel

Subject: Re: [PATCH AUTOSEL 4.19 2/3] efi: efivars: prevent double registration

On Wed, 1 Mar 2023 at 17:30, Sasha Levin <[email protected]> wrote:
>
> From: Johan Hovold <[email protected]>
>
> [ Upstream commit 0217a40d7ba6e71d7f3422fbe89b436e8ee7ece7 ]
>
> Add the missing sanity check to efivars_register() so that it is no
> longer possible to override an already registered set of efivar ops
> (without first deregistering them).
>
> This can help debug initialisation ordering issues where drivers have so
> far unknowingly been relying on overriding the generic ops.
>
> Signed-off-by: Johan Hovold <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>

NAK this is not a bugfix.

> ---
> drivers/firmware/efi/vars.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index e619ced030d52..462e88b9d2ba4 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -1195,19 +1195,28 @@ int efivars_register(struct efivars *efivars,
> const struct efivar_operations *ops,
> struct kobject *kobject)
> {
> + int rv;
> +
> if (down_interruptible(&efivars_lock))
> return -EINTR;
>
> + if (__efivars) {
> + pr_warn("efivars already registered\n");
> + rv = -EBUSY;
> + goto out;
> + }
> +
> efivars->ops = ops;
> efivars->kobject = kobject;
>
> __efivars = efivars;
>
> pr_info("Registered efivars operations\n");
> -
> + rv = 0;
> +out:
> up(&efivars_lock);
>
> - return 0;
> + return rv;
> }
> EXPORT_SYMBOL_GPL(efivars_register);
>
> --
> 2.39.2
>