Rewrite the parser for the vendor firmware descriptor with the
following improvements.
- Validate the key-value length given in a vendor descriptor
against the length of the descriptor. The current code fails
to do this and might read more bytes than available. This can
lead to out-of-bounds reads of the allocated buffer.
- Read raw data with helpers for unaligned data. This allows
the code to run on platforms that do now support unaligned memory
access by default.
- Validate the pixel limit against a default value. The default
comes from real-world devices. If the reported number of pixels
is significantly above the limit, it is likely invalid.
- Drop the obsolete print macros. There is still a warning about
invalid firmware descriptors. The rest of the output is bogus.
Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
---
drivers/gpu/drm/udl/udl_main.c | 77 ++++++++++++++++++++++------------
1 file changed, 51 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index d3a04bcb65d25..b5a6b254a2028 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -8,6 +8,8 @@
* Copyright (C) 2009 Bernie Thompson <ber...@plugable.com>
*/
+#include <linux/unaligned.h>
+
#include <drm/drm.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -23,10 +25,56 @@
#define WRITES_IN_FLIGHT (20)
#define MAX_VENDOR_DESCRIPTOR_SIZE 256
+#define UDL_SKU_PIXEL_LIMIT_DEFAULT 2080000
+
static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout);
+/*
+ * Try to make sense of whatever we parse. Therefore return @end on
+ * errors, but don't fail hard.
+ */
+static const u8 *udl_parse_key_value_pair(struct udl_device *udl, const u8
*pos, const u8 *end)
+{
+ u16 key;
+ u8 len;
+
+ /* read key */
+ if (pos >= end - 2)
+ return end;
+ key = get_unaligned_le16(pos);
+ pos += 2;
+
+ /* read value length */
+ if (pos >= end - 1)
+ return end;
+ len = *pos++;
+
+ /* read value */
+ if (pos >= end - len)
+ return end;
+ switch (key) {
+ case 0x0200: { /* maximum number of pixels */
+ unsigned int sku_pixel_limit;
+
+ if (len < sizeof(__le32))
+ break;
+ sku_pixel_limit = get_unaligned_le32(pos);
+ if (sku_pixel_limit >= 16 * UDL_SKU_PIXEL_LIMIT_DEFAULT)
+ break; /* almost 100 MiB, so probably bogus */
+ udl->sku_pixel_limit = sku_pixel_limit;
+ break;
+ }
+ default:
+ break;
+ }
+ pos += len;
+
+ return pos;
+}
+
static int udl_parse_vendor_descriptor(struct udl_device *udl)
{
+ struct drm_device *dev = &udl->drm;
struct usb_device *udev = udl_to_usb_device(udl);
void *buf;
int ret;
@@ -50,8 +98,6 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl)
desc = buf;
desc_end = desc + len;
- DRM_INFO("vendor descriptor length: %u data:%11ph\n", len, desc);
-
if ((desc[0] != len) || /* descriptor length */
(desc[1] != 0x5f) || /* vendor descriptor type */
(desc[2] != 0x01) || /* version (2 bytes) */
@@ -60,35 +106,14 @@ static int udl_parse_vendor_descriptor(struct udl_device
*udl)
goto unrecognized;
desc += 5;
- while (desc < desc_end) {
- u8 length;
- u16 key;
-
- key = le16_to_cpu(*((u16 *)desc));
- desc += sizeof(u16);
- length = *desc;
- desc++;
-
- switch (key) {
- case 0x0200: { /* max_area */
- u32 max_area = le32_to_cpu(*((u32 *)desc));
-
- DRM_DEBUG("DL chip limited to %d pixel modes\n",
- max_area);
- udl->sku_pixel_limit = max_area;
- break;
- }
- default:
- break;
- }
- desc += length;
- }
+ while (desc < desc_end)
+ desc = udl_parse_key_value_pair(udl, desc, desc_end);
goto success;
unrecognized:
/* allow udlfb to load for now even if firmware unrecognized */
- DRM_ERROR("Unrecognized vendor firmware descriptor\n");
+ drm_warn(dev, "Unrecognized vendor firmware descriptor\n");
success:
kfree(buf);
--
2.49.0
