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,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 2DF2AC10F13 for ; Thu, 11 Apr 2019 10:02:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F1B08217D4 for ; Thu, 11 Apr 2019 10:02:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726699AbfDKKCo (ORCPT ); Thu, 11 Apr 2019 06:02:44 -0400 Received: from mga17.intel.com ([192.55.52.151]:18234 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726106AbfDKKCo (ORCPT ); Thu, 11 Apr 2019 06:02:44 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.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 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 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-Type: text/plain; charset=us-ascii 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) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.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