All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] hw/uefi: security fixes
@ 2026-07-02  9:23 Gerd Hoffmann
  2026-07-02  9:23 ` [PATCH v2 1/2] hw/uefi: add sanity check Gerd Hoffmann
  2026-07-02  9:23 ` [PATCH v2 2/2] hw/uefi: disable debug function Gerd Hoffmann
  0 siblings, 2 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2026-07-02  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, peter.maydell



Gerd Hoffmann (2):
  hw/uefi: add sanity check
  hw/uefi: disable debug function

 hw/uefi/var-service-policy.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.55.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] hw/uefi: add sanity check
  2026-07-02  9:23 [PATCH v2 0/2] hw/uefi: security fixes Gerd Hoffmann
@ 2026-07-02  9:23 ` Gerd Hoffmann
  2026-07-02  9:23 ` [PATCH v2 2/2] hw/uefi: disable debug function Gerd Hoffmann
  1 sibling, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2026-07-02  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, peter.maydell

Verify the passed buffer has the minimal required length before
reading the size field + verifying the total length.

Fixes: CVE-2026-58581
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3614
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/uefi/var-service-policy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/uefi/var-service-policy.c b/hw/uefi/var-service-policy.c
index 58da4adbebaf..989bf87ddb86 100644
--- a/hw/uefi/var-service-policy.c
+++ b/hw/uefi/var-service-policy.c
@@ -276,6 +276,9 @@ static uint32_t uefi_vars_mm_check_policy_register(uefi_vars_state *uv,
     uefi_var_policy *pol;
     uint64_t length;
 
+    if (mhdr->length < sizeof(*mchk) + sizeof(*pe)) {
+        return uefi_vars_mm_policy_error(mhdr, mchk, EFI_BAD_BUFFER_SIZE);
+    }
     if (uadd64_overflow(sizeof(*mchk), pe->size, &length)) {
         return uefi_vars_mm_policy_error(mhdr, mchk, EFI_BAD_BUFFER_SIZE);
     }
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] hw/uefi: disable debug function
  2026-07-02  9:23 [PATCH v2 0/2] hw/uefi: security fixes Gerd Hoffmann
  2026-07-02  9:23 ` [PATCH v2 1/2] hw/uefi: add sanity check Gerd Hoffmann
@ 2026-07-02  9:23 ` Gerd Hoffmann
  2026-07-02 12:56   ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2026-07-02  9:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, peter.maydell

This was never meant to be active in production builds.  It's a code
path not hit on a normal boot (OVMF wouldn't try variable updates which
are not allowed), so this went unnoticed.

Wrap the call into "if (VARIABLE_POLICY_DEBUG)" so it is never used
unless the #define is changed to true.

Also fix the name printing to not overrun the entry size.

Fixes: CVE-2026-58582
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3615
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/uefi/var-service-policy.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/uefi/var-service-policy.c b/hw/uefi/var-service-policy.c
index 989bf87ddb86..b56fc62f7092 100644
--- a/hw/uefi/var-service-policy.c
+++ b/hw/uefi/var-service-policy.c
@@ -16,6 +16,8 @@
 
 #include "trace.h"
 
+#define VARIABLE_POLICY_DEBUG 0
+
 static void calc_policy(uefi_var_policy *pol);
 
 static int uefi_var_policy_post_load(void *opaque, int version_id)
@@ -40,11 +42,12 @@ const VMStateDescription vmstate_uefi_var_policy = {
 static void print_policy_entry(variable_policy_entry *pe)
 {
     uint16_t *name = (void *)pe + pe->offset_to_name;
+    uint16_t *end = (void *)pe + pe->size;
 
     fprintf(stderr, "%s:\n", __func__);
 
     fprintf(stderr, "    name ´");
-    while (*name) {
+    while (*name && name < end) {
         fprintf(stderr, "%c", *name);
         name++;
     }
@@ -173,7 +176,9 @@ efi_status uefi_vars_policy_check(uefi_vars_state *uv,
     pe = pol->entry;
 
     uefi_trace_variable(__func__, var->guid, var->name, var->name_size);
-    print_policy_entry(pe);
+    if (VARIABLE_POLICY_DEBUG) {
+        print_policy_entry(pe);
+    }
 
     if ((var->attributes & pe->attributes_must_have) != pe->attributes_must_have) {
         trace_uefi_vars_policy_deny("must-have-attr");
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] hw/uefi: disable debug function
  2026-07-02  9:23 ` [PATCH v2 2/2] hw/uefi: disable debug function Gerd Hoffmann
@ 2026-07-02 12:56   ` Philippe Mathieu-Daudé
  2026-07-06 10:49     ` Gerd Hoffmann
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-07-02 12:56 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel; +Cc: peter.maydell

Hi Gerd,

On 2/7/26 11:23, Gerd Hoffmann wrote:
> This was never meant to be active in production builds.  It's a code
> path not hit on a normal boot (OVMF wouldn't try variable updates which
> are not allowed), so this went unnoticed.
> 
> Wrap the call into "if (VARIABLE_POLICY_DEBUG)" so it is never used
> unless the #define is changed to true.
> 
> Also fix the name printing to not overrun the entry size.
> 
> Fixes: CVE-2026-58582
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3615
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/uefi/var-service-policy.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/uefi/var-service-policy.c b/hw/uefi/var-service-policy.c
> index 989bf87ddb86..b56fc62f7092 100644
> --- a/hw/uefi/var-service-policy.c
> +++ b/hw/uefi/var-service-policy.c
> @@ -16,6 +16,8 @@
>   
>   #include "trace.h"
>   
> +#define VARIABLE_POLICY_DEBUG 0

To avoid this anti-pattern, ...

>   static void calc_policy(uefi_var_policy *pol);
>   
>   static int uefi_var_policy_post_load(void *opaque, int version_id)
> @@ -40,11 +42,12 @@ const VMStateDescription vmstate_uefi_var_policy = {
>   static void print_policy_entry(variable_policy_entry *pe)
>   {
>       uint16_t *name = (void *)pe + pe->offset_to_name;
> +    uint16_t *end = (void *)pe + pe->size;
>   
>       fprintf(stderr, "%s:\n", __func__);
>   
>       fprintf(stderr, "    name ´");
> -    while (*name) {
> +    while (*name && name < end) {
>           fprintf(stderr, "%c", *name);
>           name++;
>       }
> @@ -173,7 +176,9 @@ efi_status uefi_vars_policy_check(uefi_vars_state *uv,
>       pe = pol->entry;
>   
>       uefi_trace_variable(__func__, var->guid, var->name, var->name_size);

... better keep it compiled with a trace event:

        if (trace_event_get_state_backends(TRACE_UEFI_POLICY)) {

> -    print_policy_entry(pe);
> +    if (VARIABLE_POLICY_DEBUG) {
> +        print_policy_entry(pe);
> +    }
>   
>       if ((var->attributes & pe->attributes_must_have) != pe->attributes_must_have) {
>           trace_uefi_vars_policy_deny("must-have-attr");



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] hw/uefi: disable debug function
  2026-07-02 12:56   ` Philippe Mathieu-Daudé
@ 2026-07-06 10:49     ` Gerd Hoffmann
  0 siblings, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2026-07-06 10:49 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, peter.maydell

  Hi,

> > @@ -173,7 +176,9 @@ efi_status uefi_vars_policy_check(uefi_vars_state *uv,
> >       pe = pol->entry;
> >       uefi_trace_variable(__func__, var->guid, var->name, var->name_size);
> 
> ... better keep it compiled with a trace event:
> 
>        if (trace_event_get_state_backends(TRACE_UEFI_POLICY)) {

Hmm?  The function called doesn't use trace points ...

take care,
  Gerd



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-06 10:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02  9:23 [PATCH v2 0/2] hw/uefi: security fixes Gerd Hoffmann
2026-07-02  9:23 ` [PATCH v2 1/2] hw/uefi: add sanity check Gerd Hoffmann
2026-07-02  9:23 ` [PATCH v2 2/2] hw/uefi: disable debug function Gerd Hoffmann
2026-07-02 12:56   ` Philippe Mathieu-Daudé
2026-07-06 10:49     ` Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.