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

Adriano Cordova posted 1 patch 3 weeks, 2 days ago
drivers/greybus/operation.c | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH v2] greybus: operation: fix out-of-bounds write in message allocation
Posted by Adriano Cordova 3 weeks, 2 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: 87d208feb74f ("greybus: embed message buffer into message structure")
Reported-by: syzbot+2fd6aefc361af86911d5@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=2fd6aefc361af86911d5
Cc: stable@vger.kernel.org
Assisted-by: opencode:deepseek v4 pro
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
v2: point the Fixes tag at the proper commit (87d208feb74f), and add an
    Assisted-by tag.
 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 v2] greybus: operation: fix out-of-bounds write in message allocation
Posted by Johan Hovold 3 weeks, 1 day ago
On Wed, Sep 02, 2026 at 10:59:16AM -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().

Is it really? I was under the impression the only issue here was the
potential zero-size-pointer deref.

> Reject undersized messages before parsing the message header.
> 
> Fixes: 87d208feb74f ("greybus: embed message buffer into message structure")
> Reported-by: syzbot+2fd6aefc361af86911d5@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=2fd6aefc361af86911d5
> Cc: stable@vger.kernel.org
> Assisted-by: opencode:deepseek v4 pro
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> ---
> v2: point the Fixes tag at the proper commit (87d208feb74f), and add an
>     Assisted-by tag.

Either way, Yang has already sent a fix for this as I mentioned before
(and which is now ready to be merged):

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

Johan
Re: [PATCH v2] greybus: operation: fix out-of-bounds write in message allocation
Posted by Johan Hovold 3 weeks, 1 day ago
On Fri, Sep 04, 2026 at 10:36:59AM +0200, Johan Hovold wrote:
> On Wed, Sep 02, 2026 at 10:59:16AM -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().
> 
> Is it really? I was under the impression the only issue here was the
> potential zero-size-pointer deref.

You're right of course, there's a potential small (7 byte) OOB write
here too.

Johan