From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fout-a1-smtp.messagingengine.com (fout-a1-smtp.messagingengine.com [103.168.172.144]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 98DB71EA65 for ; Sun, 14 Dec 2025 13:49:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.144 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765720170; cv=none; b=SB9jYoutqCydn1OQLm4bVLvn8ii0o54gKQgmHVjOXFjaC4w4kobnBeJHRhqn1InWruxkKCvkZljLY3lwpR++jerOcFL3N7L07kV28cVboMHik6xUs5hmrlBe4zNjwY33DDSJ4ufxWsivd67mWSa2TIa4dw+d7uEtfQ4PKfoHGLQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765720170; c=relaxed/simple; bh=dzi79dDZZOhsL0LhCVkSsC4cF2rOoLqkH1c/j2Lze78=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=dmNPpjkXFBoUuNoLWuym9UcMXilI0ETMSJEdRuzY763X+C1p5drmVUdMzCkA9CJtO01L4kcHp9oU5VHck8ez07x49vum++fXG5MSmfeOdQ567nnhCenAbOu5DWv/gy/nUHox3rEV53Uecym/Xftas9ykDf1hmDvBmKrRsar0Vsg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp; spf=pass smtp.mailfrom=sakamocchi.jp; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b=k8+nilar; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=H6br/L/u; arc=none smtp.client-ip=103.168.172.144 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b="k8+nilar"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="H6br/L/u" Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfout.phl.internal (Postfix) with ESMTP id 73EB5EC060D; Sun, 14 Dec 2025 08:49:26 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Sun, 14 Dec 2025 08:49:26 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sakamocchi.jp; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1765720166; x= 1765806566; bh=/hQIkABHefmJ0nLENLlM4Y1Q5Tc8530QPcl7Xf91WPI=; b=k 8+nilarePcjTKlhCdIPghp332y+tYyUk29jZAd9DqbMcg8LDc6EpbTCMPPjOopai h9VWoQcFj7KhsJ7Z2s0YceutRdGf+WrQsX0T11hsdr5E3rC3sy5m8ZAP4/jQI878 TOLXb7+RIVYuzfmrN3wYiFD++Pd5+n9Ss+z2cEH2xIbIpDHMzqexiJskYrVfXAtB N2IKXBUZXI5MXEPrTJJtKoeaI96xwJbkOSIGIICXd+GJNctdgTP25RGvlH2jwSQp jLHzkYhXipm2OSuwjYGu5h1KOSdxbNQT6FxAX5JWS1IkM9zOUhAxBhFoLT6AHdbo DReLF/QcWFDIPiit8HJkA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1765720166; x=1765806566; bh=/hQIkABHefmJ0nLENLlM4Y1Q5Tc8530QPcl 7Xf91WPI=; b=H6br/L/uBohcicEcrqA7yyQUfwaAxkqnqxxtSiVodD07SYV32rR Nsm30RM4VliDMG4s0qH9goiw2bUvaeGQa0XKQ38u6vZrsLwvc+gW63e3kChFmgMl MNHZVdHXe0F+mM4DLvi4PEE4Y4ey6iSzoEYatfu3fyPMiXp5eIvPEigb4WJCJlvN G+8B/NHkNT0+uC8tsg2yw8lrZVsEA0V5ZucmRX5SSH1Xv/N57jX5uSUEIGteVa0r w1WVEK7zT5zL/xErqLIprf0zvJ7dAL/wtRxQJ109GItAdC6vCoSxyLBZDPt5ew6E OhX54JRUnwUADP4Eakw9v2SN66heAYpw5eg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefgedrtddtgdefgedtlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpuffrtefokffrpgfnqfghnecuuegr ihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjug hrpeffhffvvefukfhfgggtuggjsehttdertddttddvnecuhfhrohhmpefvrghkrghshhhi ucfurghkrghmohhtohcuoehoqdhtrghkrghshhhisehsrghkrghmohgttghhihdrjhhpqe enucggtffrrghtthgvrhhnpeehhffhteetgfekvdeiueffveevueeftdelhfejieeitedv leeftdfgfeeuudekueenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehoqdhtrghkrghshhhisehsrghkrghmohgttghhihdrjhhppdhnsggprhgt phhtthhopeegpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehmohhonhgrfhhtvg hrrhgrihhnsehouhhtlhhoohhkrdgtohhmpdhrtghpthhtoheplhhinhhugidufeelgedq uggvvhgvlheslhhishhtshdrshhouhhrtggvfhhorhhgvgdrnhgvthdprhgtphhtthhope hlihhnuhigqdhkvghrnhgvlhesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthho pegurghnihhsjhhirghnghesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: ie8e14432:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 14 Dec 2025 08:49:25 -0500 (EST) Date: Sun, 14 Dec 2025 22:49:23 +0900 From: Takashi Sakamoto To: Junrui Luo Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Yuhao Jiang Subject: Re: [PATCH] firewire: core: validate response length to prevent buffer overflow Message-ID: <20251214134923.GA737872@workstation.local> Mail-Followup-To: Junrui Luo , linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Yuhao Jiang References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi, Sorry to be late for reply, but I always postpone patch review during merge window. Now 6.19-rc1 has been released, and we can start working to fix it. On Wed, Dec 03, 2025 at 10:22:32AM +0800, Junrui Luo wrote: > 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 > Reported-by: Junrui Luo > Fixes: 3038e353cfaf ("firewire: Add core firewire stack.") > Signed-off-by: Junrui Luo > --- > drivers/firewire/core-transaction.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) Thanks for your trial to fix the TODO, however I can still find an issue in your patch. > 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; > +} > + The response for lock transaction request can include data. We need to check in this case here. > 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. Thanks Takashi Sakamoto