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",