From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [78.32.30.218]) (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 E5A4A3876C3; Wed, 15 Apr 2026 19:58:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=78.32.30.218 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776283105; cv=none; b=MJewEnMzZKq3okRfGMiJVEvyvHge5h5awfpOoFDhx5clXmHsj/XnxXKxAn6VXFOoXm5JSnJg807Y+twFlpO60p9mSbkf7Zt4HMKLHeQzWi5vADVggFnxUnrDt4kz6b7TmmRhtCunuvI2BBDOG+NhoYrRNwqdZ7ATKfa6aq0SzTk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776283105; c=relaxed/simple; bh=Rn5TAguSbnyTq+0PpWIsZFRjn2Op8EOl6HKkVRq/1TE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eYWZGftz3/SNj99IxUr7aWQtsrNyUNutAw51VaSuJKKVdvT6DYWd8LtLnKgOVSzUuyp7Zhyu2PlbreU20ZW6zwXoEVo/MmWcQrVbbu1B4AZroawBfoaAJZTijmk0EP96EVYgsoCvh6I28H1uAQLSk9aHiHMWTRl+g7VXEwP9f2o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk; spf=none smtp.mailfrom=armlinux.org.uk; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b=WRxrdmIs; arc=none smtp.client-ip=78.32.30.218 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="WRxrdmIs" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=zzfItcPbm51n9sUCqPS3pMEdP67XndcXm/lQ9tFu+D0=; b=WRxrdmIsc4eRiptzsUaHuJhBKe +UVOxQ0GJxULMlm3Gx+I9D42waGwzfpRMjNhf4nurL3eYPd1HdhdlvmLK71QFhp0NqOqCtdkcrrqZ KAQhYz+iCGUUYGP4vm66y5hr6xtILN/vce8MNQ5ihZahNRmB90D3K3p5wokjUq1X27a42va07e2AE VbYPoGKiDBDRBJXwHicnFlufKVEmimawsz2detW0wGLMlD5hmTaefp64z1J/ORfEQDff6kEnmuVOV aEbLNlCkIXAzRAWynK05SMR4CFYa9jXTmb6kqUCBaFr9jIU0r5fFUjp2Xw0tQMYWxTGtQE2QtPVcz H6quAEaA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:40820) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1wD6NJ-000000002SU-0s9z; Wed, 15 Apr 2026 20:58:13 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.98.2) (envelope-from ) id 1wD6NG-000000002VN-0Po4; Wed, 15 Apr 2026 20:58:10 +0100 Date: Wed, 15 Apr 2026 20:58:09 +0100 From: "Russell King (Oracle)" To: Sam Edwards Cc: Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Alexandre Torgue , Maxime Chevallier , Ovidiu Panait , Vladimir Oltean , Baruch Siach , Serge Semin , Giuseppe Cavallaro , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH net v5] net: stmmac: Prevent NULL deref when RX memory exhausted Message-ID: References: <20260415023947.7627-1-CFSworks@gmail.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: Russell King (Oracle) On Wed, Apr 15, 2026 at 10:53:15AM -0700, Sam Edwards wrote: > On Wed, Apr 15, 2026 at 9:28 AM Russell King (Oracle) > wrote: > > > > On Wed, Apr 15, 2026 at 01:56:32PM +0100, Russell King (Oracle) wrote: > > > Locally, while debugging my issues, I used this to prevent cur_rx > > > catching up with dirty_rx: > > > > > > status = stmmac_rx_status(priv, &priv->xstats, p); > > > /* check if managed by the DMA otherwise go ahead */ > > > if (unlikely(status & dma_own)) > > > break; > > > > > > next_entry = STMMAC_NEXT_ENTRY(rx_q->cur_rx, > > > priv->dma_conf.dma_rx_size); > > > if (unlikely(next_entry == rx_q->dirty_rx)) > > > break; > > > > > > rx_q->cur_rx = next_entry; > > > > > > If we care about the cost of reloading rx_q->dirty_rx on every > > > iteration, then I'd suggest that the cost we already incur reading and > > > writing rx_q->cur_rx is something that should be addressed, and > > > eliminating that would counter the cost of reading rx_q->dirty_rx. I > > > suspect, however, that the cost is minimal, as cur_tx and dirty_rx are > > > likely in the same cache line. > > No, no, I like your approach better. :) It also removes the need for > the `limit` clamp at the top of the function, so later code can assume > limit==budget. > > > > It looks like any fix to stmmac_rx() will also need a corresponding > > > fix for stmmac_rx_zc(). > > I agree that stmmac_rx_zc() is likely also broken (in a similar way, > but not similar enough to permit a "corresponding" fix), but I don't > agree that there's a dependency relationship here. This patch is > addressing #221010, which affects the generic/non-ZC codepath; I'm > afraid the ZC codepath warrants its own investigation. The code structure is identical. The only difference is what happens to the packets. Both paths take the NAPI limit. Both paths process up to that limit of descriptors. The state saving / restoring is similar. The read_again label is the same, the condition after is the same. The ZC path differs at this point in that it will attempt to refill every 16 descriptors that have been processed. Both paths then read the descriptor and check the ownership. Both paths then increment cur_rx to point to the next entry around the ring. Both paths then get the following descriptor pointer and prefetch it. Both paths then get the extended status if we're using extended descriptors. Both paths then handle frame discard. Both paths then jump back to read_again if this isn't the last segment and we have an error. Both paths then check for error. ... and so it goes on. The ZC path to me looks like a copy-paste-and-tweak approach to adding support. The difference seems to be centered only around the handling of the data buffers in the descriptors. The overall mechanism of processing the descriptors follows the same layout in both functions. > > I have some further information, but a new curveball has just been > > chucked... and I've no idea what this will mean at this stage. Just > > take it that I won't be responding for a while. > > I think I follow your meaning. Good luck getting it straightened out! It looks like further curveballs have been thrown as a result, destroying all "plans" for the next days/week. I have aboslutely no ideas how much time or when I'll be able to look at anything at the moment, so don't assume that because I find an opportunity to send an email, everthing is back to normal. I'll also note that over the last two days I've written several emails on this, spent many hours on them, only to discard them as other ideas/research and maybe even the passage of time means they're no longer appropriate to send. Jakub: sorry, I just *can't* review stuff on netdev with everything that is going on, not when .... cna't complete this. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!