firmware: google: check if size is valid when decoding VPD data

commit 4b708b7b1a2c09fbdfff6b942ebe3a160213aacd upstream.

The VPD implementation from Chromium Vital Product Data project used to
parse data from untrusted input without checking if the meta data is
invalid or corrupted. For example, the size from decoded content may
be negative value, or larger than whole input buffer. Such invalid data
may cause buffer overflow.

To fix that, the size parameters passed to vpd_decode functions should
be changed to unsigned integer (u32) type, and the parsing of entry
header should be refactored so every size field is correctly verified
before starting to decode.

Fixes: ad2ac9d5c5e0 ("firmware: Google VPD: import lib_vpd source files")
Signed-off-by: Hung-Te Lin <hungte@chromium.org>
Cc: stable <stable@vger.kernel.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20190830022402.214442-1-hungte@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index fd5212c395c01967b8eb53e6045103de20a4efb4..34d48618f3fca437e9c068de7f1a28a8bbc102cc 100644 (file)
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -92,8 +92,8 @@ static int vpd_section_check_key_name(const u8 *key, s32 key_len)
        return VPD_OK;
 }
 
-static int vpd_section_attrib_add(const u8 *key, s32 key_len,
-                                 const u8 *value, s32 value_len,
+static int vpd_section_attrib_add(const u8 *key, u32 key_len,
+                                 const u8 *value, u32 value_len,
                                  void *arg)
 {
        int ret;

diff --git a/drivers/firmware/google/vpd_decode.c b/drivers/firmware/google/vpd_decode.c
index c62fa7063a7cfd0f366e2e17ecfc68a408c0edc9..584d0d56491f02e2673172ee6429f1056aec6d7b 100644 (file)
--- a/drivers/firmware/google/vpd_decode.c
+++ b/drivers/firmware/google/vpd_decode.c
@@ -11,8 +11,8 @@
 
 #include "vpd_decode.h"
 
-static int vpd_decode_len(const s32 max_len, const u8 *in,
-                         s32 *length, s32 *decoded_len)
+static int vpd_decode_len(const u32 max_len, const u8 *in,
+                         u32 *length, u32 *decoded_len)
 {
        u8 more;
        int i = 0;
@@ -32,18 +32,39 @@ static int vpd_decode_len(const s32 max_len, const u8 *in,
        } while (more);
 
        *decoded_len = i;
+       return VPD_OK;
+}
+
+static int vpd_decode_entry(const u32 max_len, const u8 *input_buf,
+                           u32 *_consumed, const u8 **entry, u32 *entry_len)
+{
+       u32 decoded_len;
+       u32 consumed = *_consumed;
+
+       if (vpd_decode_len(max_len - consumed, &input_buf[consumed],
+                          entry_len, &decoded_len) != VPD_OK)
+               return VPD_FAIL;
+       if (max_len - consumed < decoded_len)
+               return VPD_FAIL;
+
+       consumed += decoded_len;
+       *entry = input_buf + consumed;
+
+       /* entry_len is untrusted data and must be checked again. */
+       if (max_len - consumed < *entry_len)
+               return VPD_FAIL;
 
+       consumed += decoded_len;
+       *_consumed = consumed;
        return VPD_OK;
 }
 
-int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
+int vpd_decode_string(const u32 max_len, const u8 *input_buf, u32 *consumed,
                      vpd_decode_callback callback, void *callback_arg)
 {
        int type;
-       int res;
-       s32 key_len;
-       s32 value_len;
-       s32 decoded_len;
+       u32 key_len;
+       u32 value_len;
        const u8 *key;
        const u8 *value;
 
@@ -58,26 +79,14 @@ int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
        case VPD_TYPE_STRING:
                (*consumed)++;
 
-               /* key */
-               res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed],
-                                    &key_len, &decoded_len);
-               if (res != VPD_OK || *consumed + decoded_len >= max_len)
+               if (vpd_decode_entry(max_len, input_buf, consumed, &key,
+                                    &key_len) != VPD_OK)
                        return VPD_FAIL;
 
-               *consumed += decoded_len;
-               key = &input_buf[*consumed];
-               *consumed += key_len;
-
-               /* value */
-               res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed],
-                                    &value_len, &decoded_len);
-               if (res != VPD_OK || *consumed + decoded_len > max_len)
+               if (vpd_decode_entry(max_len, input_buf, consumed, &value,
+                                    &value_len) != VPD_OK)
                        return VPD_FAIL;
 
-               *consumed += decoded_len;
-               value = &input_buf[*consumed];
-               *consumed += value_len;
-
                if (type == VPD_TYPE_STRING)
                        return callback(key, key_len, value, value_len,
                                        callback_arg);

diff --git a/drivers/firmware/google/vpd_decode.h b/drivers/firmware/google/vpd_decode.h
index cf8c2ace155a11e1d12d1d5b9981b47b6b167325..8dbe41cac599b14b094a37d409a4a9c8d3660816 100644 (file)
--- a/drivers/firmware/google/vpd_decode.h
+++ b/drivers/firmware/google/vpd_decode.h
@@ -25,8 +25,8 @@ enum {
 };
 
 /* Callback for vpd_decode_string to invoke. */
-typedef int vpd_decode_callback(const u8 *key, s32 key_len,
-                               const u8 *value, s32 value_len,
+typedef int vpd_decode_callback(const u8 *key, u32 key_len,
+                               const u8 *value, u32 value_len,
                                void *arg);
 
 /*
@@ -44,7 +44,7 @@ typedef int vpd_decode_callback(const u8 *key, s32 key_len,
  * If one entry is successfully decoded, sends it to callback and returns the
  * result.
  */
-int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
+int vpd_decode_string(const u32 max_len, const u8 *input_buf, u32 *consumed,
                      vpd_decode_callback callback, void *callback_arg);
 
 #endif  /* __VPD_DECODE_H */