From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.migadu.com (out-122.mta1.migadu.com [95.215.58.122]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5C9D144C659 for ; Wed, 19 Aug 2026 15:10:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.122 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787152250; cv=none; b=d8H41L+VRhEzo1k0Gqlztv7AFSnA1KerDxj+/UJ5RM+4O6EhSw+t9SiuzXg13Z670kx0a35dASvvm7tDh5xp+8g5LPThKjL0ABM5udJouRS8FeGBg2QFs3JYz0VO637aNXigwLc6CuM6Qj5dAVVhvgoS9kYA/fUqerySaI4+xjE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787152250; c=relaxed/simple; bh=GspjZtFQhAFR7Oloo+MX2DOO8i0G/6TyDULbF0s6M8c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=rJpUDIvfTZbPxwBGf/lB9c+jP4MEVqDKOBUQjuYgMvd8zzDPKXN7IeCpC67xw10RBFQN6RGCY8EYdWt/fW83toeEHSqXmGQ9lM3TfJgXh4eAi17ug+oWBU1+yLg1Qnn4+d5J52lqeU97CJ+mHt5tl4b6Lvl5aq5hXIeHqKnop9g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=WQjIiVgF; arc=none smtp.client-ip=95.215.58.122 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="WQjIiVgF" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=GspjZtFQhAFR7Oloo+MX2DOO8i0G/6TyDULbF0s6M8c=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1787152245; v=1; x=1787757045; b=WQjIiVgFE0wkALi54P42h+LwxIfq4cbubaqHx3RcEzt6zIoZjsuvaiXRe02JjY7Oe1W/7VoD KKpkkbcrjGKzlWYKmgCnKSz5/NGfTfaa6eLT6uNK9N05Sqf+Tvw3I/mPUGDgMwuTm/gCA6eLFU/ 4K6i6AN8vq3TN7LuEpx4D7O8= X-Envelope-To: linux-kernel@vger.kernel.org Received: from [IPV6:2601:647:6300:a030:c8df:e66c:f17b:da8f] (2601:647:6300:a030:c8df:e66c:f17b:da8f) by smtp.migadu.com with ESMTPS id 4c90150a9e2a34a4; Wed, 19 Aug 2026 15:10:44 +0000 X-Mizu-Trace-ID: 4c90150a9e2a34a4 X-Migadu-Flow: FLOW_OUT Message-ID: Date: Wed, 19 Aug 2026 08:10:38 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net 2/2] forcedeth: stop the tx_timeout register dump past the requested window To: Marek Czernohous , netdev@vger.kernel.org, "yanjun.zhu@linux.dev" Cc: Rain River , Zhu Yanjun , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Tobias Diedrich , linux-kernel@vger.kernel.org References: <178682367884.3748309.5288746298966501007@gmail.com> <178682367886.3748309.6978554332066826294@gmail.com> From: Zhu Yanjun In-Reply-To: <178682367886.3748309.6978554332066826294@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 在 2026/8/15 12:54, Marek Czernohous 写道: > From: Marek Czernohous > > nv_tx_timeout() dumps the register window in rows of eight dwords: > > for (i = 0; i <= np->register_size; i += 32) { > netdev_info(dev, "%3x: %08x ... %08x\n", i, > readl(base + i + 0), ..., readl(base + i + 28)); > > The loop bound only checks the row's starting offset, so the final row > reads a full 32 bytes from a position that is below the end of the window > but too close to it. base is mapped with exactly that length: > > np->base = ioremap(addr, np->register_size); > > so the tail of that row is read from beyond the length the driver asked > for. Per variant, the last iteration reads past register_size by: > > NV_PCI_REGSZ_VER1 (0x270): row 0x260 reads to 0x27f, 16 bytes over > NV_PCI_REGSZ_VER2 (0x2d4): row 0x2c0 reads to 0x2df, 12 bytes over > NV_PCI_REGSZ_VER3 (0x604): row 0x600 reads to 0x61f, 28 bytes over > > This happens on every supported device, not just one of them. Note that > it is not a consequence of the sizes being odd: with i <= register_size > the offending row is reached whatever the size, and a size that were a > multiple of 32 would overrun by a full row rather than by a remainder. > > To be precise about the severity: the reads stay inside the BAR. Memory > BAR sizes are powers of two, the driver only accepts a region with > pci_resource_len() >= register_size (forcedeth.c:5757-5762), and the > next power of two at or above each register_size already covers the > offending row: 0x400 for 0x270 and 0x2d4, 0x800 for 0x604. ioremap() > also rounds the mapped length up to page granularity, so the reads land > inside the mapping the CPU has as well. What they leave is the window > the driver asked for, not the BAR and not the mapping. That is still a > driver reading registers it did not ask for, and it is trivial to > avoid, but nobody should expect a fault from it. > > Changing <= to < is not enough: register_size is a length and every size > above is larger than its last row start, so i still reaches the offending > row. Check that the whole row fits instead. > > The trade-off is that a partial trailing row is no longer dumped: 16 bytes > for VER1, 20 for VER2, 4 for VER3. That seemed preferable to reading > outside the requested window, and to open-coding a second, narrower dump > for the remainder in what is a debug-only path. Extending the dump to > cover the tail can be done on top if anyone misses those registers. > > Only reachable with the debug_tx_timeout module parameter, which defaults > to false. It has not been observed at runtime: forcing a genuine TX > timeout on the reference machine is not something I can do safely, so this > rests on the arithmetic above and on a build test, not on a reproduction. > UBSAN does not catch it either, since these are MMIO reads rather than an > array access. It was found by reading the function while fixing the > saved_config_space off-by-one in nv_suspend() and nv_resume(). > > The dump was introduced with a fixed 0x400 bound while ioremap() mapped > only NV_PCI_REGSZ (0x270), so it read about 0x190 bytes too far from the > start. Commit 86a0f04387bf ("[PATCH] forcedeth: fix initialization") > later replaced 0x400 with np->register_size, which shrank the overrun to > the remainder but did not remove it. > > Fixes: c2dba06dae7d ("[PATCH] forcedeth: rewritten tx irq handling") > Signed-off-by: Marek Czernohous Thanks a lot. Reviewed-by: Zhu Yanjun Zhu Yanjun > Assisted-by: Claude:claude-opus-5 > --- > drivers/net/ethernet/nvidia/forcedeth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c > index dc804e111564..f0218a0eab5c 100644 > --- a/drivers/net/ethernet/nvidia/forcedeth.c > +++ b/drivers/net/ethernet/nvidia/forcedeth.c > @@ -2740,7 +2740,7 @@ static void nv_tx_timeout(struct net_device *dev, unsigned int txqueue) > > netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr); > netdev_info(dev, "Dumping tx registers\n"); > - for (i = 0; i <= np->register_size; i += 32) { > + for (i = 0; i + 32 <= np->register_size; i += 32) { > netdev_info(dev, > "%3x: %08x %08x %08x %08x " > "%08x %08x %08x %08x\n", -- Best Regards, Yanjun.Zhu