lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 11 Apr 2019 13:02:35 +0300
From:   Andriy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Jacob Pan <jacob.jun.pan@...ux.intel.com>
Cc:     iommu@...ts.linux-foundation.org,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        David Woodhouse <dwmw2@...radead.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        Yi Liu <yi.l.liu@...el.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        Raj Ashok <ashok.raj@...el.com>,
        Christoph Hellwig <hch@...radead.org>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        "Liu, Yi L" <yi.l.liu@...ux.intel.com>, Liu@...le.fi.intel.com,
        Eric Auger <eric.auger@...hat.com>
Subject: Re: [PATCH 08/18] iommu: Introduce cache_invalidate API

On Wed, Apr 10, 2019 at 02:21:31PM -0700, Jacob Pan wrote:
> On Tue, 9 Apr 2019 20:37:55 +0300
> Andriy Shevchenko <andriy.shevchenko@...ux.intel.com> 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 <andriy.shevchenko@...ux.intel.com> 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


Powered by blists - more mailing lists