Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6489cc44.5d0a0220.47312.6e31@mx.google.com>
Date:   Wed, 14 Jun 2023 09:35:02 +0200
From:   Christian Marangi <ansuelsmth@gmail.com>
To:     Jose Abreu <Jose.Abreu@synopsys.com>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [net-next PATCH v5 4/5] net: ethernet: stmicro: stmmac: generate
 stmmac dma conf before open
References: <20220723142933.16030-1-ansuelsmth@gmail.com>
 <20220723142933.16030-5-ansuelsmth@gmail.com>
 <DM4PR12MB508882D5BE351BD756A7A9A4D35AA@DM4PR12MB5088.namprd12.prod.outlook.com>
 <64898745.5d0a0220.546a.a6f1@mx.google.com>
 <DM4PR12MB508899B25BA18E2E53939BE7D35AA@DM4PR12MB5088.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM4PR12MB508899B25BA18E2E53939BE7D35AA@DM4PR12MB5088.namprd12.prod.outlook.com>
Precedence: bulk

On Wed, Jun 14, 2023 at 01:48:40PM +0000, Jose Abreu wrote:
> From: Christian Marangi <ansuelsmth@gmail.com>
> Date: Wed, Jun 14, 2023 at 03:16:08
> 
> > I'm not following the meaning of leak here. If it's intended as a memory
> > leak then dma_conf is correctly freed in the 2 user of __stmmac_open.
> > 
> > stmmac_init_phy also doesn't seems to use dma_conf. Am I missing
> > something here?
> > 
> 
> Sorry, I should have been clearer: It's not leaking the dma_conf per-se but
> the contents of it: The DMA descriptors. Since the memcpy() is only done after
> init_phy(); if init_phy() fails, then stmmac will never free up the DMA descriptors.
> 
> Does it make sense?
> 

Thanks for the clarification! Sent a follow-up patch that should fix the
possible leak. Would be good if you can also test it for a Tested-by
tag.

-- 
	Ansuel