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

[Yang Zi]

gb_connection_recv() reads msg_size from the received message header but
only rejects it when it is larger than the received buffer
("size < msg_size"); it does not reject msg_size smaller than the message
header itself.  A malicious or corrupted header.size of 0 passes that
check and is forwarded to gb_operation_create_incoming() with size 0.

There, request_size = size - sizeof(struct gb_operation_msg_hdr)
underflows to SIZE_MAX - 7, and in gb_operation_message_alloc()
message_size = payload_size + sizeof(*header) wraps back around to 0.
The "message_size > hd->buffer_size_max" check is therefore bypassed,
kzalloc(0) returns ZERO_SIZE_PTR, and gb_operation_message_init() writes
header->size to that pointer.

KASAN report:

    BUG: KASAN: null-ptr-deref in gb_operation_message_init drivers/greybus/operation.c:340 [inline] [greybus]
    BUG: KASAN: null-ptr-deref in gb_operation_message_alloc+0xab4/0xdb0 drivers/greybus/operation.c:385 [greybus]
    Write of size 2 at addr 0000000000000010 by task syz.0.1/1100

Fix this by rejecting messages whose claimed size is smaller than the
message header in gb_connection_recv(), and additionally make
gb_operation_message_alloc() overflow-safe by comparing the payload size
against hd->buffer_size_max - sizeof(*header) before adding the header
size.

Signed-off-by: Yang Zi <2959243019@xxxxxx>
---
 drivers/greybus/operation.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 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",