[Libvir] PATCH: Fix dir/fs storage pool when SELinux is disabled
When SELinux is disabled fgetfilecon() may well return -1, if a file has no extended attribute with security context data. This causes the storage pool to skip that file. The fix is to check whether errno is ENODATA and treat that as an expected error case & ignore it. Dan. Index: src/storage_backend.c =================================================================== RCS file: /data/cvs/libvirt/src/storage_backend.c,v retrieving revision 1.8 diff -u -p -r1.8 storage_backend.c --- src/storage_backend.c 27 Feb 2008 10:37:19 -0000 1.8 +++ src/storage_backend.c 14 Mar 2008 21:21:05 -0000 @@ -240,17 +240,22 @@ virStorageBackendUpdateVolInfoFD(virConn #if HAVE_SELINUX if (fgetfilecon(fd, &filecon) == -1) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot get file context of %s: %s"), - vol->target.path, strerror(errno)); - return -1; - } - vol->target.perms.label = strdup(filecon); - if (vol->target.perms.label == NULL) { - virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("context")); - return -1; + if (errno != ENODATA) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot get file context of %s: %s"), + vol->target.path, strerror(errno)); + return -1; + } else { + vol->target.perms.label = NULL; + } + } else { + vol->target.perms.label = strdup(filecon); + if (vol->target.perms.label == NULL) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("context")); + return -1; + } + freecon(filecon); } - freecon(filecon); #else vol->target.perms.label = NULL; #endif -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Mar 14, 2008 at 09:23:09PM +0000, Daniel P. Berrange wrote:
When SELinux is disabled fgetfilecon() may well return -1, if a file has no extended attribute with security context data. This causes the storage pool to skip that file. The fix is to check whether errno is ENODATA and treat that as an expected error case & ignore it.
Looks fine, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
"Daniel P. Berrange" <berrange@redhat.com> wrote:
When SELinux is disabled fgetfilecon() may well return -1, if a file has no extended attribute with security context data. This causes the storage pool to skip that file. The fix is to check whether errno is ENODATA and treat that as an expected error case & ignore it.
Hi Dan, That code should handle ENOTSUP as well as ENODATA. Here's the change: Treat ENOTSUP like ENODATA, after failed fgetfilecon. * src/storage_backend.c (virStorageBackendUpdateVolInfoFD): Treat a failed fgetfilecon with errno == ENOTSUP the same as for ENODATA. diff --git a/src/storage_backend.c b/src/storage_backend.c index 9702de3..4a58cb6 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -240,7 +240,7 @@ virStorageBackendUpdateVolInfoFD(virConnectPtr conn, #if HAVE_SELINUX if (fgetfilecon(fd, &filecon) == -1) { - if (errno != ENODATA) { + if (errno != ENODATA && errno != ENOTSUP) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot get file context of %s: %s"), vol->target.path, strerror(errno)); -- 1.5.4.4.482.g16f99
On Mon, Mar 17, 2008 at 05:36:49PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
When SELinux is disabled fgetfilecon() may well return -1, if a file has no extended attribute with security context data. This causes the storage pool to skip that file. The fix is to check whether errno is ENODATA and treat that as an expected error case & ignore it.
Hi Dan,
That code should handle ENOTSUP as well as ENODATA.
Ok, comitted that too Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Mon, Mar 17, 2008 at 05:36:49PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
When SELinux is disabled fgetfilecon() may well return -1, if a file has no extended attribute with security context data. This causes the storage pool to skip that file. The fix is to check whether errno is ENODATA and treat that as an expected error case & ignore it.
Hi Dan,
That code should handle ENOTSUP as well as ENODATA.
Ok, comitted that too
No problem. I've just done it.
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering