From a83ec37232ca1ea817b3446b905f9e880223de21 Mon Sep 17 00:00:00 2001

From: Yu Watanabe <watanabe.yu+github@gmail.com>

Date: Tue, 6 Dec 2022 13:06:57 +0900

Subject: [PATCH] boot: cleanups for efivar_get() and friends

- rename function arguments for storing results, and support the case

  that they are NULL,

- return earlier on error,

- always validate read size in efivar_get_uint32_le() and efivar_get_uint64_le().

(cherry picked from commit 9e406b1141da2d93b73428910f2504850631a3ee)

Related: #2141979

---

 src/boot/efi/util.c | 63 ++++++++++++++++++++++++---------------------

 src/boot/efi/util.h |  6 ++---

 2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c

index 57436dbf0c..3eba2ade07 100644

--- a/src/boot/efi/util.c

+++ b/src/boot/efi/util.c

@@ -91,7 +91,7 @@ EFI_STATUS efivar_set_uint64_le(const EFI_GUID *vendor, const char16_t *name, ui

         return efivar_set_raw(vendor, name, buf, sizeof(buf), flags);

 }

-EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **value) {

+EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **ret) {

         _cleanup_free_ char16_t *buf = NULL;

         EFI_STATUS err;

         char16_t *val;

@@ -108,12 +108,12 @@ EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **v

         if ((size % sizeof(char16_t)) != 0)

                 return EFI_INVALID_PARAMETER;

-        if (!value)

+        if (!ret)

                 return EFI_SUCCESS;

         /* Return buffer directly if it happens to be NUL terminated already */

         if (size >= sizeof(char16_t) && buf[size / sizeof(char16_t) - 1] == 0) {

-                *value = TAKE_PTR(buf);

+                *ret = TAKE_PTR(buf);

                 return EFI_SUCCESS;

         }

@@ -123,18 +123,17 @@ EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **v

         memcpy(val, buf, size);

         val[size / sizeof(char16_t) - 1] = 0; /* NUL terminate */

-        *value = val;

+        *ret = val;

         return EFI_SUCCESS;

 }

-EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name, UINTN *i) {

+EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name, UINTN *ret) {

         _cleanup_free_ char16_t *val = NULL;

         EFI_STATUS err;

         uint64_t u;

         assert(vendor);

         assert(name);

-        assert(i);

         err = efivar_get(vendor, name, &val;;

         if (err != EFI_SUCCESS)

@@ -143,7 +142,8 @@ EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name,

         if (!parse_number16(val, &u, NULL) || u > UINTN_MAX)

                 return EFI_INVALID_PARAMETER;

-        *i = u;

+        if (ret)

+                *ret = u;

         return EFI_SUCCESS;

 }

@@ -156,15 +156,17 @@ EFI_STATUS efivar_get_uint32_le(const EFI_GUID *vendor, const char16_t *name, ui

         assert(name);

         err = efivar_get_raw(vendor, name, &buf, &size);

-        if (err == EFI_SUCCESS && ret) {

-                if (size != sizeof(uint32_t))

-                        return EFI_BUFFER_TOO_SMALL;

+        if (err != EFI_SUCCESS)

+                return err;

+        if (size != sizeof(uint32_t))

+                return EFI_BUFFER_TOO_SMALL;

+

+        if (ret)

                 *ret = (uint32_t) buf[0] << 0U | (uint32_t) buf[1] << 8U | (uint32_t) buf[2] << 16U |

                         (uint32_t) buf[3] << 24U;

-        }

-        return err;

+        return EFI_SUCCESS;

 }

 EFI_STATUS efivar_get_uint64_le(const EFI_GUID *vendor, const char16_t *name, uint64_t *ret) {

@@ -176,19 +178,21 @@ EFI_STATUS efivar_get_uint64_le(const EFI_GUID *vendor, const char16_t *name, ui

         assert(name);

         err = efivar_get_raw(vendor, name, &buf, &size);

-        if (err == EFI_SUCCESS && ret) {

-                if (size != sizeof(uint64_t))

-                        return EFI_BUFFER_TOO_SMALL;

+        if (err != EFI_SUCCESS)

+                return err;

+

+        if (size != sizeof(uint64_t))

+                return EFI_BUFFER_TOO_SMALL;

+        if (ret)

                 *ret = (uint64_t) buf[0] << 0U | (uint64_t) buf[1] << 8U | (uint64_t) buf[2] << 16U |

                         (uint64_t) buf[3] << 24U | (uint64_t) buf[4] << 32U | (uint64_t) buf[5] << 40U |

                         (uint64_t) buf[6] << 48U | (uint64_t) buf[7] << 56U;

-        }

-        return err;

+        return EFI_SUCCESS;

 }

-EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **buffer, UINTN *size) {

+EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **ret, UINTN *ret_size) {

         _cleanup_free_ char *buf = NULL;

         UINTN l;

         EFI_STATUS err;

@@ -200,16 +204,15 @@ EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **b

         buf = xmalloc(l);

         err = RT->GetVariable((char16_t *) name, (EFI_GUID *) vendor, NULL, &l, buf);

-        if (err == EFI_SUCCESS) {

-

-                if (buffer)

-                        *buffer = TAKE_PTR(buf);

+        if (err != EFI_SUCCESS)

+                return err;

-                if (size)

-                        *size = l;

-        }

+        if (ret)

+                *ret = TAKE_PTR(buf);

+        if (ret_size)

+                *ret_size = l;

-        return err;

+        return EFI_SUCCESS;

 }

 EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, bool *ret) {

@@ -219,13 +222,15 @@ EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, b

         assert(vendor);

         assert(name);

-        assert(ret);

         err = efivar_get_raw(vendor, name, &b, &size);

-        if (err == EFI_SUCCESS)

+        if (err != EFI_SUCCESS)

+                return err;

+

+        if (ret)

                 *ret = *b > 0;

-        return err;

+        return EFI_SUCCESS;

 }

 void efivar_set_time_usec(const EFI_GUID *vendor, const char16_t *name, uint64_t usec) {

diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h

index b33c50f9fc..994cf52ad6 100644

--- a/src/boot/efi/util.h

+++ b/src/boot/efi/util.h

@@ -105,9 +105,9 @@ EFI_STATUS efivar_set_uint32_le(const EFI_GUID *vendor, const char16_t *NAME, ui

 EFI_STATUS efivar_set_uint64_le(const EFI_GUID *vendor, const char16_t *name, uint64_t value, uint32_t flags);

 void efivar_set_time_usec(const EFI_GUID *vendor, const char16_t *name, uint64_t usec);

-EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **value);

-EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **buffer, UINTN *size);

-EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name, UINTN *i);

+EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **ret);

+EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **ret, UINTN *ret_size);

+EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name, UINTN *ret);

 EFI_STATUS efivar_get_uint32_le(const EFI_GUID *vendor, const char16_t *name, uint32_t *ret);

 EFI_STATUS efivar_get_uint64_le(const EFI_GUID *vendor, const char16_t *name, uint64_t *ret);

 EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, bool *ret);