[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Redundant lstats in libxl_pvusb.c




>>> On 4/7/2016 at 06:43 PM, in message
<22278.14817.248393.423722@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
<Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> Chun Yan Liu writes ("Re: Redundant lstats in libxl_pvusb.c"): 
> > <22274.33583.712655.413448@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson 
> > <Ian.Jackson@xxxxxxxxxxxxx> wrote:  
> > > In libxl_usb.c, usbintf_get_drvpath calls stat(2) on the driver sysfs  
> > > path, and then realpath on the same path.  
> >  
> > It's true. This could be done by calling realpath only. Will correct. 
>  
> Thanks. 
>  
> > > And bind_usbintf calls stat(2) on the driver directory path, and then  
> > > open(2) on a file in that directory.  
> >  
> > It's not true. It calls stat(2) on a file in driver path  
> (driver/interface), 
> > and open(2) on another file in that driver path (driver/bind). 
>  
> I have read the function again and you are right. 
>  
> Coverity said: 
>  
> > > > >>>     CID 1358111: Security best practices violations (TOCTOU) 
> > > > >>>     Calling function "open" that uses "path" after a check 
> > > > >>>     function. This can cause a time-of-check, time-of-use 
> > > > >>>     race condition. 
>  
> But it seems that it is confused by the reuse of the path variable. 
> I think this is arguably a bug in Coverity. 
>  
> But, evidently, the same reuse confused me too.  Maybe we should turn 
> `path' into two variables, `intf_path' and `bind_path' ?  What do you 
> think ? 

Yeah, maybe it's better to change into 'intf_path' and 'bind_path', I'll update.
But it's unavoidable that some temp variable will be reused for many
times.

Chunyan

>  
> Thanks, 
> Ian. 
>  
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.