[PATCH] greybus: reject messages with claimed size smaller than header

[Zhao Feng <zhaofeng2@cmss.chinamobile.com>]

gb_connection_recv() performs two sanity checks on received
messages: the receive buffer must hold at least a full message
header, and it must hold at least the message size claimed in the
header.  Neither check verifies that the claimed message size
covers the header itself.

A message claiming a size smaller than sizeof(struct
gb_operation_msg_hdr) (e.g. 7) passes both checks when 8 or more
bytes were received, and the claimed size is then used as the
message size:

- gb_operation_create_incoming() computes
  request_size = msg_size - sizeof(header), which underflows to
  SIZE_MAX.  The request handler then reads payload from a message
  whose payload size is SIZE_MAX; syzbot reported this as a KASAN
  global-buffer-overflow read in gb_svc_request_handler().
- gb_connection_recv_response() similarly computes
  payload_size = size - sizeof(*header) for accepted short
  responses, which underflows the same way.

Reject such messages up front in gb_connection_recv().  A claimed
size equal to the header size is valid (header only, no payload),
so the comparison must be strict.

Reported-by: syzbot+693652ded53d3eb9cb91@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=693652ded53d3eb9cb91
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
---
No Fixes tag: the missing validation dates back to the original
greybus import; the introducing commit could not be verified with
the trees available to me.

Not runtime-tested (no greybus hardware).  The affected function is
identical in current upstream master, so the patch applies there
as-is.

This also answers the question raised on the syzbot thread about
whether the underflow is reachable: it is, because the existing
checks compare the *received* size against the claimed size, never
the claimed size against the header size.

 drivers/greybus/operation.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index 8459e9bc07..83b16c78f7 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -1046,6 +1046,12 @@ 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 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.39.2