[PATCH v2] greybus: operation: Fix NULL pointer dereference in gb_operation_message_alloc()

[PATCH v2] greybus: operation: Fix NULL pointer dereference in gb_operation_message_alloc()

[Yang Zi]

gb_connection_recv() accepts a received message whose advertised size is
smaller than struct gb_operation_msg_hdr.  In particular, a header with a
size of zero passes the incomplete-message check and reaches
gb_operation_create_incoming().

This issue was found using a locally modified syzkaller.  The
analysis and fix were assisted by GPT-5.6.

The subtraction used to derive the request payload size then underflows.
When gb_operation_message_alloc() adds the header size, the result wraps
to zero, bypassing the maximum-buffer-size check.  kzalloc(0) returns
ZERO_SIZE_PTR and gb_operation_message_init() subsequently dereferences
it.

Reject advertised sizes smaller than the message header.  Also check the
payload size before adding the header size, so that the size calculation
cannot wrap and bypass the buffer-size limit.

Fixes: 87d208feb74f ("greybus: embed message buffer into message structure")
Assisted-by: Codex:gpt-5.6
Signed-off-by: Yang Zi <2959243019@qq.com>
---
v2:
- Add the syzkaller provenance and Assisted-by trailer.
- Regenerate the patch for git-send-email.
- Verify the received patch with git am and checkpatch.

 drivers/greybus/operation.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index 7e12ffb2dd60..c3d51176c373 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -364,14 +364,21 @@ gb_operation_message_alloc(struct gb_host_device *hd, u8 type,
 {
 	struct gb_message *message;
 	struct gb_operation_msg_hdr *header;
-	size_t message_size = payload_size + sizeof(*header);
+	size_t message_size;
 
-	if (message_size > hd->buffer_size_max) {
+	/*
+	 * Reject a payload size that would make the total message size
+	 * overflow, before it wraps around and bypasses the maximum
+	 * buffer size check.
+	 */
+	if (payload_size > hd->buffer_size_max - sizeof(*header)) {
 		dev_warn(&hd->dev, "requested message size too big (%zu > %zu)\n",
-			 message_size, hd->buffer_size_max);
+			 payload_size, hd->buffer_size_max - sizeof(*header));
 		return NULL;
 	}
 
+	message_size = payload_size + sizeof(*header);
+
 	/* Allocate the message structure and buffer. */
 	message = kmem_cache_zalloc(gb_message_cache, gfp_flags);
 	if (!message)
@@ -1047,6 +1054,11 @@ 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: short message received (%zu < %zu)\n",
+				    connection->name, 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.50.1

Re: [PATCH v2] greybus: operation: Fix NULL pointer dereference in gb_operation_message_alloc()

[Johan Hovold]

On Fri, Aug 28, 2026 at 06:00:04PM +0800, Yang Zi wrote:
> gb_connection_recv() accepts a received message whose advertised size is
> smaller than struct gb_operation_msg_hdr.  In particular, a header with a
> size of zero passes the incomplete-message check and reaches
> gb_operation_create_incoming().

Thanks for the fix.

Please shorten the commit summary (Subject) to something less verbose,
like:

	greybus: operation: fix NULL-deref on short request

> This issue was found using a locally modified syzkaller.  The
> analysis and fix were assisted by GPT-5.6.

This can go at the end of the commit message.

> The subtraction used to derive the request payload size then underflows.
> When gb_operation_message_alloc() adds the header size, the result wraps
> to zero, bypassing the maximum-buffer-size check.  kzalloc(0) returns
> ZERO_SIZE_PTR and gb_operation_message_init() subsequently dereferences
> it.
> 
> Reject advertised sizes smaller than the message header.  Also check the
> payload size before adding the header size, so that the size calculation
> cannot wrap and bypass the buffer-size limit.

This is arguably two changes; a fix for the NULL-deref due to the
missing header sanity check and a hardening against any further bugs
like it.

But I guess they can go in together in one patch like you do here.

> Fixes: 87d208feb74f ("greybus: embed message buffer into message structure")

This is not the commit that introduced the issue. This should be:

Fixes: d90c25b0a279 ("greybus: let operation layer examine incoming data")

as that's the commit that started acting on the header size field
without first rejecting invalid headers (even if there was a WARN_ON()
at that time).

> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Yang Zi <2959243019@qq.com>
> ---
> v2:
> - Add the syzkaller provenance and Assisted-by trailer.
> - Regenerate the patch for git-send-email.
> - Verify the received patch with git am and checkpatch.
> 
>  drivers/greybus/operation.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
> index 7e12ffb2dd60..c3d51176c373 100644
> --- a/drivers/greybus/operation.c
> +++ b/drivers/greybus/operation.c
> @@ -364,14 +364,21 @@ gb_operation_message_alloc(struct gb_host_device *hd, u8 type,
>  {
>  	struct gb_message *message;
>  	struct gb_operation_msg_hdr *header;
> -	size_t message_size = payload_size + sizeof(*header);
> +	size_t message_size;
>  
> -	if (message_size > hd->buffer_size_max) {
> +	/*
> +	 * Reject a payload size that would make the total message size
> +	 * overflow, before it wraps around and bypasses the maximum
> +	 * buffer size check.
> +	 */

I think you can drop the comment.

> +	if (payload_size > hd->buffer_size_max - sizeof(*header)) {
>  		dev_warn(&hd->dev, "requested message size too big (%zu > %zu)\n",

And this should now say "payload size".

> -			 message_size, hd->buffer_size_max);
> +			 payload_size, hd->buffer_size_max - sizeof(*header));
>  		return NULL;
>  	}
>  
> +	message_size = payload_size + sizeof(*header);
> +
>  	/* Allocate the message structure and buffer. */
>  	message = kmem_cache_zalloc(gb_message_cache, gfp_flags);
>  	if (!message)
> @@ -1047,6 +1054,11 @@ 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: short message received (%zu < %zu)\n",
> +				    connection->name, 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",

Johan