From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756851AbcLUPDK (ORCPT ); Wed, 21 Dec 2016 10:03:10 -0500 Received: from mail-bl2nam02on0072.outbound.protection.outlook.com ([104.47.38.72]:21028 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751294AbcLUPDG (ORCPT ); Wed, 21 Dec 2016 10:03:06 -0500 X-Greylist: delayed 4732 seconds by postgrey-1.27 at vger.kernel.org; Wed, 21 Dec 2016 10:03:05 EST From: "Mintz, Yuval" To: Colin King , "netdev@vger.kernel.org" CC: "linux-kernel@vger.kernel.org" , "Elior, Ariel" , "Tayar, Tomer" Subject: RE: [PATCH][V2] qed: fix memory leak of a qed_spq_entry on error failure paths Thread-Topic: [PATCH][V2] qed: fix memory leak of a qed_spq_entry on error failure paths Thread-Index: AQHSWraF2ob/Bz0D3Ei/hjfvfzGeUKESY79A Date: Wed, 21 Dec 2016 13:29:56 +0000 Message-ID: References: <20161220114424.11244-1-colin.king@canonical.com> In-Reply-To: <20161220114424.11244-1-colin.king@canonical.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Yuval.Mintz@cavium.com; x-originating-ip: [31.168.140.228] x-microsoft-exchange-diagnostics: 1;CO2PR07MB2615;7:nlLZa79iYLmJPkXAMThwr+pLZ4ROet6c8bYUomL/d335fjASif3KFt/VCdk4RUbwNaJeyjTpMnEeqkoYyc4Q/atWR8sf6KLqklCXFv8vmurLbzIUuA2cNN2mpqF94f1k7Y6+fUmgYeiVXChmypQQOZigPsMYDrVpyIZ7+Yit5lYFNF4Ssd78LM1N08AdM1/qy7hj0TMH2+gaBDED7i+Qw5nzCs2/KyWQxizFvtKDfCx6O/c3pLc0M0XacyXIQ/2C1pqE/LUPRuwUhFhLUMrLMu2ZAjH7eP6n2zRqox4uacrv6To2SomMXKZU8eFH2SBBIJM3tlDPmei7FYRSuy0nhhZo0+Wfxp/BkK+b2mdhDT5wup+Evg8VtzPhWcPstmw+Ni891pD6DE0CdEY3J8dHXFrvIWbU5LUJXY2kCOZ+WW25YiCNXUZgR1UX+HmGQKvQL5dtVXh1OlrEfxKMn6xrLQ== x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39450400003)(199003)(189002)(99286002)(2906002)(3660700001)(38730400001)(97736004)(8936002)(6436002)(3280700002)(106116001)(106356001)(105586002)(2501003)(5001770100001)(2950100002)(122556002)(68736007)(76576001)(2900100001)(92566002)(5660300001)(229853002)(3846002)(7736002)(101416001)(7696004)(50986999)(76176999)(54356999)(102836003)(9686002)(33656002)(86362001)(189998001)(107886002)(25786008)(8676002)(77096006)(4001430100002)(66066001)(6506006)(6116002)(4326007)(74316002)(81156014)(305945005)(81166006);DIR:OUT;SFP:1101;SCL:1;SRVR:CO2PR07MB2615;H:BL2PR07MB2306.namprd07.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; x-ms-office365-filtering-correlation-id: f5c380ab-3c5c-49b1-e819-08d429a574ac x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:CO2PR07MB2615; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(198206253151910); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6041248)(20161123560025)(20161123564025)(20161123562025)(20161123555025)(6072148);SRVR:CO2PR07MB2615;BCL:0;PCL:0;RULEID:;SRVR:CO2PR07MB2615; x-forefront-prvs: 01630974C0 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: cavium.com X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Dec 2016 13:29:56.6190 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO2PR07MB2615 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id uBLF3OnS003495 > 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