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@xxxxxx>
> ---
> 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