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

[Eric Blake]

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).

Probably worth a v3, so we get virStorageBackendVolOpen right.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100521/608506a4/attachment-0001.sig>