From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andriy Shevchenko Subject: Re: [PATCH 08/18] iommu: Introduce cache_invalidate API Date: Thu, 11 Apr 2019 13:02:35 +0300 Message-ID: <20190411100235.GQ9224@smile.fi.intel.com> References: <1554767973-30125-1-git-send-email-jacob.jun.pan@linux.intel.com> <1554767973-30125-9-git-send-email-jacob.jun.pan@linux.intel.com> <20190409100718.GE9224@smile.fi.intel.com> <20190409094328.03731c3c@jacob-builder> <20190409173755.GX9224@smile.fi.intel.com> <20190410142131.50ee2e44@jacob-builder> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20190410142131.50ee2e44@jacob-builder> Sender: linux-kernel-owner@vger.kernel.org To: Jacob Pan Cc: iommu@lists.linux-foundation.org, LKML , Joerg Roedel , David Woodhouse , Alex Williamson , Jean-Philippe Brucker , Yi Liu , "Tian, Kevin" , Raj Ashok , Christoph Hellwig , Lu Baolu , "Liu, Yi L" , Liu@smile.fi.intel.com, Eric Auger List-Id: iommu@lists.linux-foundation.org On Wed, Apr 10, 2019 at 02:21:31PM -0700, Jacob Pan wrote: > On Tue, 9 Apr 2019 20:37:55 +0300 > Andriy Shevchenko wrote: > > On Tue, Apr 09, 2019 at 09:43:28AM -0700, Jacob Pan wrote: > > > On Tue, 9 Apr 2019 13:07:18 +0300 > > > Andriy Shevchenko wrote: > > > > On Mon, Apr 08, 2019 at 04:59:23PM -0700, Jacob Pan wrote: > > > > > > > +int iommu_cache_invalidate(struct iommu_domain *domain, struct > > > > > device *dev, > > > > > + struct iommu_cache_invalidate_info > > > > > *inv_info) +{ > > > > > + int ret = 0; > > > > > > > > Redundant assignment. > > > > > > > I am not a security expert but initialization of local variable can > > > be more secure. > > > I was looking at this talk. > > > https://outflux.net/slides/2018/lss/danger.pdf > > > https://cwe.mitre.org/data/definitions/457.html > > > > I hardly see any of these applied to your case here. > > Care to show what I'm missing? > > > I thought your comments was that I should not need to initialize local > variable ret = 0. Correct. > Always initialize local variable can be a good > security practice as suggested in the paper. Perhaps I missed > something :) Paper suggested to do that in a sense to avoid use of uninitialized variable. This is not your case (usually it's not the case for variable which contains return code), so, assignment is redundant. Moreover, default assignment can hide an actual warning and an issue. Security people are not always correct. > > > > > + if (unlikely(!domain->ops->cache_invalidate)) > > > > > + return -ENODEV; > > > > > + > > > > > + ret = domain->ops->cache_invalidate(domain, dev, > > > > > inv_info); + > > > > > + return ret; > > > > > +} -- With Best Regards, Andy Shevchenko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5844BC10F13 for ; Thu, 11 Apr 2019 10:03:00 +0000 (UTC) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2CF8A217D4 for ; Thu, 11 Apr 2019 10:03:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2CF8A217D4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id E3BAC240D; Thu, 11 Apr 2019 10:02:59 +0000 (UTC) Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 1626223CC for ; Thu, 11 Apr 2019 10:02:46 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7307AF4 for ; Thu, 11 Apr 2019 10:02:45 +0000 (UTC) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Apr 2019 03:02:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,337,1549958400"; d="scan'208";a="133343941" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.86]) by orsmga008.jf.intel.com with ESMTP; 11 Apr 2019 03:02:36 -0700 Received: from andy by smile with local (Exim 4.92) (envelope-from ) id 1hEWXT-0000iE-67; Thu, 11 Apr 2019 13:02:35 +0300 Date: Thu, 11 Apr 2019 13:02:35 +0300 From: Andriy Shevchenko To: Jacob Pan Subject: Re: [PATCH 08/18] iommu: Introduce cache_invalidate API Message-ID: <20190411100235.GQ9224@smile.fi.intel.com> References: <1554767973-30125-1-git-send-email-jacob.jun.pan@linux.intel.com> <1554767973-30125-9-git-send-email-jacob.jun.pan@linux.intel.com> <20190409100718.GE9224@smile.fi.intel.com> <20190409094328.03731c3c@jacob-builder> <20190409173755.GX9224@smile.fi.intel.com> <20190410142131.50ee2e44@jacob-builder> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190410142131.50ee2e44@jacob-builder> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.10.1 (2018-07-13) Cc: "Liu, Yi L" , "Tian, Kevin" , Raj Ashok , Jean-Philippe Brucker , iommu@lists.linux-foundation.org, LKML , Alex Williamson , Liu@smile.fi.intel.com, David Woodhouse X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Sender: iommu-bounces@lists.linux-foundation.org Errors-To: iommu-bounces@lists.linux-foundation.org Message-ID: <20190411100235.ar496m2ogHhW1n0fJA86DIRRaQ-x56HVX2VyFAgPQjc@z> On Wed, Apr 10, 2019 at 02:21:31PM -0700, Jacob Pan wrote: > On Tue, 9 Apr 2019 20:37:55 +0300 > Andriy Shevchenko wrote: > > On Tue, Apr 09, 2019 at 09:43:28AM -0700, Jacob Pan wrote: > > > On Tue, 9 Apr 2019 13:07:18 +0300 > > > Andriy Shevchenko wrote: > > > > On Mon, Apr 08, 2019 at 04:59:23PM -0700, Jacob Pan wrote: > > > > > > > +int iommu_cache_invalidate(struct iommu_domain *domain, struct > > > > > device *dev, > > > > > + struct iommu_cache_invalidate_info > > > > > *inv_info) +{ > > > > > + int ret = 0; > > > > > > > > Redundant assignment. > > > > > > > I am not a security expert but initialization of local variable can > > > be more secure. > > > I was looking at this talk. > > > https://outflux.net/slides/2018/lss/danger.pdf > > > https://cwe.mitre.org/data/definitions/457.html > > > > I hardly see any of these applied to your case here. > > Care to show what I'm missing? > > > I thought your comments was that I should not need to initialize local > variable ret = 0. Correct. > Always initialize local variable can be a good > security practice as suggested in the paper. Perhaps I missed > something :) Paper suggested to do that in a sense to avoid use of uninitialized variable. This is not your case (usually it's not the case for variable which contains return code), so, assignment is redundant. Moreover, default assignment can hide an actual warning and an issue. Security people are not always correct. > > > > > + if (unlikely(!domain->ops->cache_invalidate)) > > > > > + return -ENODEV; > > > > > + > > > > > + ret = domain->ops->cache_invalidate(domain, dev, > > > > > inv_info); + > > > > > + return ret; > > > > > +} -- With Best Regards, Andy Shevchenko _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu