Re: [PATCH 08/18] iommu: Introduce cache_invalidate API

[Jacob Pan]

On Tue, 9 Apr 2019 20:37:55 +0300
Andriy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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. Always initialize local variable can be a good
security practice as suggested in the paper. Perhaps I missed
something :)
> > > > +	if (unlikely(!domain->ops->cache_invalidate))
> > > > +		return -ENODEV;
> > > > +
> > > > +	ret = domain->ops->cache_invalidate(domain, dev,
> > > > inv_info); +
> > > > +	return ret;
> > > > +}    
> 

[Jacob Pan]