[v1] greybus: operation: fix out-of-bounds write in message allocation

[PATCH] greybus: operation: fix out-of-bounds write in message allocation

Posted by Adriano Cordova 3 weeks, 3 days ago

The incoming message size from the device header (header.size) is
trusted without checking that it is at least the size of the message
header itself, but a value smaller than sizeof(struct gb_operation_msg_hdr)
underflows request_size in gb_operation_create_incoming(), wraps around
in gb_operation_message_alloc(), and results in a tiny buffer that is
then written past its end in gb_operation_message_init().

Reject undersized messages before parsing the message header.

Fixes: d37b1db13f8b ("greybus: refactor gb_connection_recv()")
Reported-by: syzbot+2fd6aefc361af86911d5@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=2fd6aefc361af86911d5
Cc: stable@vger.kernel.org
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 drivers/greybus/operation.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index 7e12ffb2dd..df6daee4fb 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -1047,6 +1047,14 @@ void gb_connection_recv(struct gb_connection *connection,
 	/* Use memcpy as data may be unaligned */
 	memcpy(&header, data, sizeof(header));
 	msg_size = le16_to_cpu(header.size);
+	if (msg_size < sizeof(header)) {
+		dev_err_ratelimited(dev,
+				    "%s: malformed message 0x%04x of type 0x%02x received (%zu < %zu)\n",
+				    connection->name,
+				    le16_to_cpu(header.operation_id),
+				    header.type, msg_size, sizeof(header));
+		return;
+	}
 	if (size < msg_size) {
 		dev_err_ratelimited(dev,
 				    "%s: incomplete message 0x%04x of type 0x%02x received (%zu < %zu)\n",
-- 
2.51.0

Re: [PATCH] greybus: operation: fix out-of-bounds write in message allocation

Posted by Johan Hovold 3 weeks, 3 days ago

On Tue, Sep 01, 2026 at 11:30:16PM -0400, Adriano Cordova wrote:
> The incoming message size from the device header (header.size) is
> trusted without checking that it is at least the size of the message
> header itself, but a value smaller than sizeof(struct gb_operation_msg_hdr)
> underflows request_size in gb_operation_create_incoming(), wraps around
> in gb_operation_message_alloc(), and results in a tiny buffer that is
> then written past its end in gb_operation_message_init().
> 
> Reject undersized messages before parsing the message header.
> 
> Fixes: d37b1db13f8b ("greybus: refactor gb_connection_recv()")

This isn't the commit that introduced the issue (even if that commit
made things worse by removing a WARN_ON()).

> Reported-by: syzbot+2fd6aefc361af86911d5@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=2fd6aefc361af86911d5
> Cc: stable@vger.kernel.org

How did you find and fix this issue? Are you missing an Assisted-by tag
here?

> Signed-off-by: Adriano Cordova <adrianox@gmail.com>

Note that this is also being fixed here:

	https://lore.kernel.org/lkml/tencent_1CB189506DEFA4B38099D654FE6C2A2F2E07@qq.com/

Johan