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",
On 11/8/26 7:33 PM, Zhao Feng wrote:
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.
So the problem occurs when a message arrives that has a bogus header. We know we received enough bytes to hold a
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",
On 11/8/26 7:33 PM, Zhao Feng wrote:
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.
So the problem occurs when a message arrives that has a bogus header. We verify that the bytes received are enough to hold a gb_operation_msg_hdr structure. But the 2-byte size field within that structure lies at the beginning, and there was previously no check that its value made sense.
A valid header->size must be at least sizeof(header) (for a zero-byte payload). It looks like gb_connection_recv_response() might handle a header whose size field is too small, but gb_connection_recv_request() does not.
In any case, I think this fix looks good.
Reviewed-by: Alex Elder elder@riscstar.com
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",
-
Alex Elder -
Zhao Feng