linux-kernel - [PATCH] firewire: core: validate response length to prevent buffer overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID:
 <SYBPR01MB7881ABA3D4B9129FC2E6A3E4AFD9A@SYBPR01MB7881.ausprd01.prod.outlook.com>
Date: Wed, 03 Dec 2025 10:22:32 +0800
From: Junrui Luo <moonafterrain@...look.com>
To: Takashi Sakamoto <o-takashi@...amocchi.jp>, 
 Stefan Richter <stefanr@...6.in-berlin.de>, 
 Kristian Høgsberg <krh@...hat.com>
Cc: linux1394-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org, 
 Yuhao Jiang <danisjiang@...il.com>, Junrui Luo <moonafterrain@...look.com>
Subject: [PATCH] firewire: core: validate response length to prevent buffer
 overflow

The FireWire core transaction handling code does not validate that
the length of a READ_BLOCK_RESPONSE matches the length originally
requested in the READ_BLOCK_REQUEST. A malicious FireWire device
could respond with more data than requested, causing a buffer overflow
in the callback handler when the response data is copied into the
caller's buffer.

This issue has been acknowledged by a FIXME comment:
  "FIXME: sanity check packet, is length correct, does tcodes
   and addresses match to the transaction request queried later."

Fix this by validating the response length against the original request
length before passing data to the callback.

Reported-by: Yuhao Jiang <danisjiang@...il.com>
Reported-by: Junrui Luo <moonafterrain@...look.com>
Fixes: 3038e353cfaf ("firewire: Add core firewire stack.")
Signed-off-by: Junrui Luo <moonafterrain@...look.com>
---
 drivers/firewire/core-transaction.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index c65f491c54d0..52f05e8f3798 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -1095,11 +1095,23 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
 }
 EXPORT_SYMBOL(fw_core_handle_request);
 
+static size_t get_request_data_length(const struct fw_packet *request)
+{
+	int request_tcode = async_header_get_tcode(request->header);
+
+	if (request_tcode == TCODE_READ_QUADLET_REQUEST)
+		return 4;
+	else if (request_tcode == TCODE_READ_BLOCK_REQUEST)
+		return async_header_get_data_length(request->header);
+	return 0;
+}
+
 void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 {
 	struct fw_transaction *t = NULL, *iter;
 	u32 *data;
 	size_t data_length;
+	size_t request_length;
 	int tcode, tlabel, source, rcode;
 
 	tcode = async_header_get_tcode(p->header);
@@ -1107,9 +1119,6 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	source = async_header_get_source(p->header);
 	rcode = async_header_get_rcode(p->header);
 
-	// FIXME: sanity check packet, is length correct, does tcodes
-	// and addresses match to the transaction request queried later.
-	//
 	// For the tracepoints event, let us decode the header here against the concern.
 
 	switch (tcode) {
@@ -1160,6 +1169,13 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 		return;
 	}
 
+	request_length = get_request_data_length(&t->packet);
+	if (request_length > 0 && data_length > request_length) {
+		fw_notice(card, "response length (%zu) exceeds request length (%zu) from node %x, truncating\n",
+			  data_length, request_length, source);
+		data_length = request_length;
+	}
+
 	/*
 	 * The response handler may be executed while the request handler
 	 * is still pending.  Cancel the request handler.

---
base-commit: 4a26e7032d7d57c998598c08a034872d6f0d3945
change-id: 20251203-fixes-760722565563

Best regards,
-- 
Junrui Luo <moonafterrain@...look.com>