From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.tipi-net.de (mail.tipi-net.de [194.13.80.246]) (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 8851838B7A6; Fri, 24 Apr 2026 10:40:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=194.13.80.246 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777027256; cv=none; b=NbWZMmhyjyvaMNPWV4M8xvmkUKx6cMGK0LSdppmFB8lAOtBrhcK+aEywF8g1sz08JmRyukkmsp2nGRWphnbLHxwJs8EX+CENRtySCpgnglnnUr144AdAtmw0/4KLyFxkp+S28TkXfG9jgKXkRwu2CDGiQ8vZ7t0wCNqUN5eQ1Zg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777027256; c=relaxed/simple; bh=ra0spWUlavd514SMAgukniVNiGyiBJxnDLGWIrKLAF4=; h=MIME-Version:Date:From:To:Cc:Subject:In-Reply-To:References: Message-ID:Content-Type; b=hNoywO/l+AdNADfkPmJn5ij/EmCy+Hk2oC3mQXb/zNCuIAUxh4eAtnIR+0XZJhof3FHKIu1ekbIfpkvGpiks9fErhsMv3A7Y57snrmvgMwZ+PMGzYdLlT7BkxTVDDYi+/xkDkAQiP6DNPG8FhdnWkQBgIEVyzCMyVEiSf26uY2o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tipi-net.de; spf=pass smtp.mailfrom=tipi-net.de; dkim=pass (2048-bit key) header.d=tipi-net.de header.i=@tipi-net.de header.b=XpcOZbkv; arc=none smtp.client-ip=194.13.80.246 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tipi-net.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tipi-net.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tipi-net.de header.i=@tipi-net.de header.b="XpcOZbkv" Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id DFB1CA3C20; Fri, 24 Apr 2026 12:30:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tipi-net.de; s=dkim; t=1777026648; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=vjSNcas3jlFNxzs2kV/iQLiDHeyejuhQ73BS26aMrqQ=; b=XpcOZbkv3qb3uEFOeHObHiYznmzpFnQxIClQpmlGv+qmxmz/+6R4MfTAlfsd0IOKtcROtl kvUQyA99CO8D3ROOLbslmNyR1xcuWkXE4nWmA6LSWkwp2gM+X+kKggk2NeIHiCji6rC4U0 iYGVyjQDXs/Qvcyy4h/lAjvck834AgUVCZie01BS2XHnzRdWbkeuPWUxkRiQ/vRer7I64I Dc1R6DYY5nMwL/fX56pnshGRnKGxz71X7E8Ng4vCps3G5zk4oUzyKJ9YQ4KhnY/ntY3gTV fmD+calkQvnxPZ/oR1Ha07kq0NvVbNTdBhMpbCPBwZpKmPKdTh/IvoLPKPvUCA== Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Fri, 24 Apr 2026 12:30:36 +0200 From: Nicolai Buchwitz To: =?UTF-8?Q?Th=C3=A9o_Lebrun?= Cc: Nicolas Ferre , Claudiu Beznea , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Haavard Skinnemoen , Jeff Garzik , Paolo Valerio , Conor Dooley , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Vladimir Kondratiev , Gregory CLEMENT , =?UTF-8?Q?Beno=C3=AEt_Monin?= , Tawfik Bayouk , Thomas Petazzoni , Maxime Chevallier Subject: Re: [PATCH net] net: macb: drop in-flight Tx SKBs on close In-Reply-To: <20260424-macb-drop-tx-v1-1-b3ecb787d84d@bootlin.com> References: <20260424-macb-drop-tx-v1-1-b3ecb787d84d@bootlin.com> Message-ID: <1030f36978e299123c914917a15b934b@tipi-net.de> X-Sender: nb@tipi-net.de Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 Hi Théo, thanks for your patch. On 24.4.2026 12:01, Théo Lebrun wrote: > [...] > > for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) { > + dropped = CIRC_CNT(queue->tx_head, queue->tx_tail, > + bp->tx_ring_size); > + queue->stats.tx_dropped += dropped; > + bp->dev->stats.tx_dropped += dropped; AFAIUI CIRC_CNT counts descriptor slots, not packets. A fragmented skb uses multiple slots so tx_dropped would be overcounted? > + > + for (tail = queue->tx_tail; tail != queue->tx_head; tail++) > + macb_tx_unmap(bp, macb_tx_skb(queue, tail), 0); I might be missing something, but couldn't this crash on the macb_alloc_consistent() -> out_err path after a previous close with in-flight frames? 1. First close: the new loop runs and frees skbs, but tx_head and tx_tail are not reset. kfree(tx_skb) sets it to NULL. 2. Second open: macb_alloc_consistent() fails early (e.g. the tx dma_alloc_coherent on the first line) and jumps to out_err. 3. macb_free_consistent() runs again. CIRC_CNT is non-zero (stale from previous session). macb_tx_skb() dereferences queue->tx_skb which is NULL. Or if the failure happens later, the loop would iterate over a freshly kmalloc'd (uninitialized) tx_skb array and macb_tx_unmap() would read garbage mapping/skb pointers. Maybe reset tx_head = tx_tail = 0 after the loop, or guard with if (queue->tx_skb)? > + > kfree(queue->tx_skb); > queue->tx_skb = NULL; > queue->tx_ring = NULL; > > [...] Thanks, Nicolai