Skip to contentSkip to conversation

firmware: raspberrypi: Remove VLA usage

PATCH New Kees Cook 2 participants 4 messages Jun 29, 2018 – Jul 5, 2018 1 message not in the archive Broadcom BCM2711/BCM2835 Arm architecture

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 firmware: raspberrypi: Remove VLA usage

In the quest to remove all stack VLA usage from the kernel[1], this removes the VLA in favor of a maximum size and adds a sanity check. Existing callers of the firmware interface never need more than 24 bytes (struct gpio_set_config). This chooses 32 just to stay ahead of future growth.

[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com

Signed-off-by Kees Cook <[email protected]>
---
 drivers/firmware/raspberrypi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/firmware/raspberrypi.c +7 -1
index 0602626bf72d..b80f15214b73 100644
@@ -21,6 +21,8 @@
 #define MBOX_DATA28(msg)		((msg) & ~0xf)
 #define MBOX_CHAN_PROPERTY		8
 
+#define MAX_RPI_FW_PROP_BUF_SIZE	32
+
 static struct platform_device *rpi_hwmon;
 
 struct rpi_firmware {

@@ -145,11 +147,15 @@ int rpi_firmware_property(struct rpi_firmware *fw,
 	/* Single tags are very small (generally 8 bytes), so the
 	 * stack should be safe.
 	 */
-	u8 data[buf_size + sizeof(struct rpi_firmware_property_tag_header)];
+	u8 data[sizeof(struct rpi_firmware_property_tag_header) +
+		MAX_RPI_FW_PROP_BUF_SIZE];
 	struct rpi_firmware_property_tag_header *header =
 		(struct rpi_firmware_property_tag_header *)data;
 	int ret;
 
+	if (WARN_ON(buf_size > sizeof(data) - sizeof(*header)))
+		return -EINVAL;
+
 	header->tag = tag;
 	header->buf_size = buf_size;
 	header->req_resp_size = 0;
Signature · 6 lines
2.17.1


-- 
Kees Cook
Pixel Security

A message that is not in the archive (sent off-list or not yet received): Message-ID: <20180629184449.GA37304@beast

#2 lore raw

PATCH 1/1 fixup! firmware: raspberrypi: Remove VLA usage

Kees - with this fix to your patch, the kernel boots again (otherwise, the FW would try to parse the uninitialized bits of stack and throw errors). If you're good with me squashing this in, I'll do so and send it to -next.

Signed-off-by Eric Anholt <[email protected]>
---
 drivers/firmware/raspberrypi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/raspberrypi.c +1 -1
index b80f15214b73..a200a2174611 100644
@@ -162,7 +162,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
 	memcpy(data + sizeof(struct rpi_firmware_property_tag_header),
 	       tag_data, buf_size);
 
-	ret = rpi_firmware_property_list(fw, &data, sizeof(data));
+	ret = rpi_firmware_property_list(fw, &data, buf_size + sizeof(*header));
 	memcpy(tag_data,
 	       data + sizeof(struct rpi_firmware_property_tag_header),
 	       buf_size);
Signature · 1 line
2.18.0
author
#3 lore raw

Re: [PATCH] fixup! firmware: raspberrypi: Remove VLA usage

Quoted 15 lines · Eric Anholt

Kees - with this fix to your patch, the kernel boots again (otherwise, the FW would try to parse the uninitialized bits of stack and throw errors). If you're good with me squashing this in, I'll do so and send it to -next.

Signed-off-by Eric Anholt <[email protected]>
---
 drivers/firmware/raspberrypi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/raspberrypi.c +0 -0
index b80f15214b73..a200a2174611 100644

drivers/firmware/raspberrypi.c int rpi_firmware_property(struct rpi_firmware *fw, · line 168

Quoted 8 lines
drivers/firmware/raspberrypi.c +1 -1
@@ -162,7 +162,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
        memcpy(data + sizeof(struct rpi_firmware_property_tag_header),
               tag_data, buf_size);
 
-       ret = rpi_firmware_property_list(fw, &data, sizeof(data));
+       ret = rpi_firmware_property_list(fw, &data, buf_size + sizeof(*header));
        memcpy(tag_data,
               data + sizeof(struct rpi_firmware_property_tag_header),
               buf_size);

Eek! Yes, thank you. Yeah, please feel free to squash. Thanks for testing!

-Kees

Signature · 2 lines
Kees Cook
Pixel Security
#4 lore raw

Re: [PATCH] fixup! firmware: raspberrypi: Remove VLA usage

Quoted 19 lines · Kees Cook
Quoted 18 lines · Eric Anholt

Kees - with this fix to your patch, the kernel boots again (otherwise, the FW would try to parse the uninitialized bits of stack and throw errors). If you're good with me squashing this in, I'll do so and send it to -next.

Signed-off-by Eric Anholt <[email protected]>
---
 drivers/firmware/raspberrypi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/raspberrypi.c +0 -0
index b80f15214b73..a200a2174611 100644
@@ -162,7 +162,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
        memcpy(data + sizeof(struct rpi_firmware_property_tag_header),
               tag_data, buf_size);
Quoted 7 lines

drivers/firmware/raspberrypi.c int rpi_firmware_property(struct rpi_firmware *fw, · line 168

Quoted 5 lines
drivers/firmware/raspberrypi.c +1 -1
-       ret = rpi_firmware_property_list(fw, &data, sizeof(data));
+       ret = rpi_firmware_property_list(fw, &data, buf_size + sizeof(*header));
        memcpy(tag_data,
               data + sizeof(struct rpi_firmware_property_tag_header),
               buf_size);

Eek! Yes, thank you. Yeah, please feel free to squash. Thanks for testing!

Squashed and sent the PR. Thanks!