[libvirt] [PATCH 3/3] storage: Check for invalid storage mode before opening
Eric Blake
eblake at redhat.com
Thu May 20 22:25:54 UTC 2010
On 05/20/2010 01:23 PM, 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
> pools can then proceed along, ignoring the invalid files.
stat() before open() is racy. Better yet is using the
O_NONBLOCK|O_NOCTTY flags of open(); gnulib guarantees they are defined,
and I recently lobbied POSIX to guarantee that it will be safe on all
platforms (it is already safe on Linux):
http://austingroupbugs.net/view.php?id=141
>
> +int
> +virStorageBackendVolOpen(const char *path)
> +{
> + int fd;
> + struct stat sb;
> +
> + if (stat(path, &sb) < 0) {
> + virReportSystemError(errno,
> + _("cannot stat file '%s'"), path);
> + return -1;
> + }
> +
> + if (!S_ISREG(sb.st_mode) &&
> + !S_ISCHR(sb.st_mode) &&
> + !S_ISBLK(sb.st_mode)) {
Regular files and block devices I can understand, but character devices?
That includes things like /dev/zero and /dev/null; do those really make
sense as the backing of a volume store?
> + VIR_DEBUG("Skipping volume path %s, unexpected file mode.", path);
> + return -2;
> + }
> +
> + if ((fd = open(path, O_RDONLY)) < 0) {
> + virReportSystemError(errno,
> + _("cannot open volume '%s'"),
> + path);
In other words, I'd rather see open(path,O_RDONLY|O_NONBLOCK|O_NOCTTY)
then fstat(), and not your stat()/open() sequence.
But the rest of the patch looks sane.
--
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/20100520/ab12927a/attachment-0001.sig>
More information about the libvir-list
mailing list