Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760035AbcLUO2u (ORCPT ); Wed, 21 Dec 2016 09:28:50 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:53145 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752715AbcLUO2q (ORCPT ); Wed, 21 Dec 2016 09:28:46 -0500 Subject: Re: [PATCH][V2] qed: fix memory leak of a qed_spq_entry on error failure paths To: "Mintz, Yuval" , "netdev@vger.kernel.org" References: <20161220114424.11244-1-colin.king@canonical.com> Cc: "linux-kernel@vger.kernel.org" , "Elior, Ariel" , "Tayar, Tomer" From: Colin Ian King Message-ID: <2f9e0ee7-4c59-1b4b-ebeb-13ca52aa912a@canonical.com> Date: Wed, 21 Dec 2016 14:28:01 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1426 Lines: 33 On 21/12/16 13:29, Mintz, Yuval wrote: >> From: Colin Ian King >> >> A qed_spq_entry entry is allocated by qed_sp_init_request but is not kfree'd >> if an error occurs, causing a memory leak. Fix this by returning the previously >> allocated spq entry and also setting *pp_ent to NULL to be safe. >> >> Thanks to Yuval Mintz for suggestions on how to improve my original fix. >> >> Signed-off-by: Colin Ian King > > We've given it a more thorough look, and apparently this isn't the correct fix. > So I'll start by saying sorry for making you send this V2 needlessly. > > It boils down to the fact there are two kinds of SPQ entries - > Those originating from the 'free_pool' and those from the 'unlimited_pending'. > Only those originating from the free_pool should be returned > using the qed_spq_return_entry(), as only those actually point to a valid > dma-mapped memory where FW expects to find the entries; > Returning the other kind would lead to assertions later, > as driver would post a ramrod to FW which actually points to address 0. > > Looking at the error flows, it seems possible this isn't the only faulty > error flow in the SPQ. I suggest you'd drop this and we'll take it from > here [although if you really have the urge to continue - please do]. > > Thanks, > Yuval > > Sure, lets drop my fixes, I'm out of time on this for 2016 anyhow. Colin