From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758165AbcFANgv (ORCPT ); Wed, 1 Jun 2016 09:36:51 -0400 Received: from mail-bl2on0111.outbound.protection.outlook.com ([65.55.169.111]:12640 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752956AbcFANgs convert rfc822-to-8bit (ORCPT ); Wed, 1 Jun 2016 09:36:48 -0400 From: Yuval Mintz To: Arnd Bergmann , David Miller CC: Manish Chopra , Sudarsana Kalluru , netdev , linux-kernel , Ariel Elior Subject: RE: [PATCH v2] qed: fix qed_fill_link() error handling Thread-Topic: [PATCH v2] qed: fix qed_fill_link() error handling Thread-Index: AQHRvAmG7mRr56x8akesSTifUK0xy5/Um+PA Date: Wed, 1 Jun 2016 13:36:38 +0000 Message-ID: References: <1464623197-2084229-1-git-send-email-arnd@arndb.de> <8209894.K472MDjuEr@wuerfel> <11393820.LSE4DkNrNd@wuerfel> In-Reply-To: <11393820.LSE4DkNrNd@wuerfel> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: arndb.de; dkim=none (message not signed) header.d=none;arndb.de; dmarc=none action=none header.from=qlogic.com; x-originating-ip: [31.168.140.228] x-ms-office365-filtering-correlation-id: d74e6930-5f73-4f54-9b44-08d38a21c21b x-microsoft-exchange-diagnostics: 1;CY1PR1101MB1147;5:BsTxrDtzoRzH2yMlcUo+kOdDMcs8ShMvL3KYJqIL/f0QLW1fD8QxeGWuo7xNyCWHyw391T2085Z8wWl2aP2VhP9bBYHuG/FqLUq/cBAc9d8YycXOpkCZT/ONL7hSQOAzGrTh4ZDjLsXqHWVyT2X5YA==;24:RiT5fZczwce2FLX05QYVUuSqLltCXyP5Veln02MlXu3QqTySoQgX4rzsQj7LFWHO91opbi8MpMUzTzgzpNoAjtyVvvUi7rOCazUB69kAS1I=;7:AI15k4Q0cyk2PvUIryw3TQmVKpphUpw/2x0cpPTbQXr3DXr/i12hKLPVDDxqcQAvUE004NNU2nYWKRYKTGmQWJjVD0NQNn7ztjUW07h5ZHWZNcfTT2OT6MBSBp7s3AgEO35P07UkLy+b/BJjjtFRFFL/VoSliuXtUhXsxJ01VqqNehKEaNzF95i60bhzk2iq;20:nNVZcv3+3jO8KuxK7+xwkAAkVH76en1wbeO9TZZINMugQM5eb+RlT+z0hldxMginoXcdSUnDm1VaZbQgmCFtsBjpyax6hH6QXAj5zUHw8uOAR7IBaDT+gdFPpfX3uDFRvZp3lHrXyhXlVQqy3Ls9dnwv3KQ9+WGXe2Pf7YxX8aA= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR1101MB1147; x-ld-processed: 0d68a1f9-1490-4d0e-8767-a87dab3ef2ba,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:CY1PR1101MB1147;BCL:0;PCL:0;RULEID:;SRVR:CY1PR1101MB1147; x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(24454002)(377454003)(586003)(74316001)(10400500002)(86362001)(66066001)(6116002)(102836003)(11100500001)(3280700002)(76576001)(4326007)(76176999)(2906002)(54356999)(4001430100002)(9686002)(50986999)(3846002)(92566002)(2950100001)(2900100001)(5004730100002)(93886004)(122556002)(77096005)(99286002)(5008740100001)(8936002)(107886002)(19580405001)(189998001)(19580395003)(33656002)(8676002)(81166006)(87936001)(5003600100002)(3660700001)(106116001)(5001770100001)(5002640100001);DIR:OUT;SFP:1102;SCL:1;SRVR:CY1PR1101MB1147;H:CO2PR11MB0088.namprd11.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: qlogic.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2016 13:36:38.1858 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 0d68a1f9-1490-4d0e-8767-a87dab3ef2ba X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR1101MB1147 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > gcc warns about qed_fill_link possibly accessing uninitialized data: > > drivers/net/ethernet/qlogic/qed/qed_main.c: In function 'qed_fill_link': > drivers/net/ethernet/qlogic/qed/qed_main.c:1170:35: error: 'link_caps' may be > used uninitialized in this function [-Werror=maybe-uninitialized] > > While this warning is only about the specific case of CONFIG_QED_SRIOV being > disabled but the function getting called for a VF (which should never happen), > another possibility is that qed_mcp_get_*() fails without returning data. > > This rearranges the code so we bail out in either of the two cases and print a > warning instead of accessing the uninitialized data. > > The qed_link_output structure remains untouched in this case, but all callers first > call memset() on it, so at least we are not leaking stack data then. > > As discussed, we also use a compile-time check to ensure we never use any of > the VF code if CONFIG_QED_SRIOV is disabled, and the PCI device table is > updated to no longer bind to virtual functions in that configuration. > > Signed-off-by: Arnd Bergmann > > --- > On Wednesday, June 1, 2016 11:10:30 AM CEST Yuval Mintz wrote: > > Actually, I think VF probe should gracefully fail in that case, as > > qed_vf_hw_prepare() would simply return -EINVAL. > > But I can honestly say I've never tested this flow, and I agree > > there's no reason to allow VF probe in case we're not supporting SRIOV. > > ok > > > So I guess removing the PCI ID and defining IS_PF to be true in case > > CONFIG_QED_SRIOV isn't set is the right way to go. > > Do you want to revise your patch, or do you want me to do it? > > I've done the patch below now, please either Ack or modify it the way you like > and forward it. > > Thanks, > > Arnd Perhaps it would have made more sense as a 2-part series; But I'm content with the changes themselves. I'd let Dave decide whether he wants it split. Thanks for taking the time fixing this. Acked-by: Yuval Mintz