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

Re: [libvirt] [PATCH v2] storage: Check for invalid storage mode before opening



On 05/21/2010 02:17 PM, Eric Blake wrote:
> On 05/21/2010 11:03 AM, Cole Robinson wrote:
>> If a directory pool contains pipes or sockets, a pool start can fail or hang:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=589577
>>
>> We already try to avoid these special files, but only attempt after
>> opening the path, which is where the problems lie. Unify volume opening
>> into a single function which runs stat() before any open() call. Directory
> 
> This sentence of the commit log is out-of-date...
> 
>> pools can then proceed along, ignoring the invalid files.
>>
>> v2: Use ATTRIBUTE_NONNULL. Drop stat check, just open with
>>     O_NONBLOCK|O_NOCTTY.
> 
> ...given this sentence.
> 
>> +int
>> +virStorageBackendVolOpen(const char *path)
>> +{
>> +    int fd;
>> +
>> +    if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
>> +        virReportSystemError(errno,
>> +                             _("cannot open volume '%s'"),
>> +                             path);
>> +        return -1;
>> +    }
> 
> I'm thinking that we still want to use fstat() (after the open()) and
> reject directories, sockets, and pipes (or more accurately, accept only
> regular files, block devices, and possibly character devices).
> 
> In other words, you got rid of the stat/open race (good), but also got
> rid of the strictness provided by validating [f]stat (not so good).
> 

Sorry, I should have pointed out that there is already code that does
this, see virStorageBackendUpdateVolTargetInfoFD, which is always called
after VolOpen. The existing code is all confusingly arranged though, so
maybe I should take a harder look at organizing.

- Cole


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