[libvirt] [PATCH] ignore SELinuxSetFilecon error in SELinuxSetSecurityFileLabel if on nfs

If virDomainAttachDevice() was called with an image that was located on a root-squashed NFS server, and in a directory that was unreadable by root on the machine running libvirtd, the attach would fail due to an attempt to change the selinux label of the image with EACCES (which isn't covered as an ignore case in SELinuxSetFilecon()) NFS doesn't support SELinux labelling anyway, so we mimic the failure handling of commit 93a18bbafaf11729d3ca1241e11bee133d77fa77, which just ignores the errors if the target is on an NFS filesystem (in SELinuxSetSecurityAllLabel() only, though.) This can be seen as a follow-on to commit 347d266c51705f4987fa5ce2a0ecb314ed8745ce, which ignores file open failures of files on NFS that occur directly in virDomainDiskDefForeachPath() (also necessary), but does not ignore failures in functions that are called from there (eg SELinuxSetSecurityFileLabel()). --- src/security/security_selinux.c | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7dd9b14..a9b63ea 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -453,20 +453,27 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, void *opaque) { const virSecurityLabelDefPtr secdef = opaque; + int ret; if (depth == 0) { if (disk->shared) { - return SELinuxSetFilecon(path, default_image_context); + ret = SELinuxSetFilecon(path, default_image_context); } else if (disk->readonly) { - return SELinuxSetFilecon(path, default_content_context); + ret = SELinuxSetFilecon(path, default_content_context); } else if (secdef->imagelabel) { - return SELinuxSetFilecon(path, secdef->imagelabel); + ret = SELinuxSetFilecon(path, secdef->imagelabel); } else { return 0; } } else { - return SELinuxSetFilecon(path, default_content_context); + ret = SELinuxSetFilecon(path, default_content_context); } + if (ret < 0 && + virStorageFileIsSharedFSType(path, + VIR_STORAGE_FILE_SHFS_NFS) != 1) + return ret; + else + return 0; } static int -- 1.7.1

On 11/10/2010 12:52 PM, Laine Stump wrote:
If virDomainAttachDevice() was called with an image that was located on a root-squashed NFS server, and in a directory that was unreadable by root on the machine running libvirtd, the attach would fail due to an attempt to change the selinux label of the image with EACCES (which isn't covered as an ignore case in SELinuxSetFilecon())
NFS doesn't support SELinux labelling anyway, so we mimic the failure handling of commit 93a18bbafaf11729d3ca1241e11bee133d77fa77, which just ignores the errors if the target is on an NFS filesystem (in SELinuxSetSecurityAllLabel() only, though.)
+ if (ret < 0 && + virStorageFileIsSharedFSType(path, + VIR_STORAGE_FILE_SHFS_NFS) != 1) + return ret; + else + return 0;
I had to scratch my head on this one. It might be easier to read as: if (ret < 0 && virStorageFileIsSharedFSType(path, VIR_STORAGE_FILE_SHFS_NFS) == 1) return 0; return ret; ACK, with that tweak. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/10/2010 03:35 PM, Eric Blake wrote:
On 11/10/2010 12:52 PM, Laine Stump wrote:
If virDomainAttachDevice() was called with an image that was located on a root-squashed NFS server, and in a directory that was unreadable by root on the machine running libvirtd, the attach would fail due to an attempt to change the selinux label of the image with EACCES (which isn't covered as an ignore case in SELinuxSetFilecon())
NFS doesn't support SELinux labelling anyway, so we mimic the failure handling of commit 93a18bbafaf11729d3ca1241e11bee133d77fa77, which just ignores the errors if the target is on an NFS filesystem (in SELinuxSetSecurityAllLabel() only, though.)
+ if (ret< 0&& + virStorageFileIsSharedFSType(path, + VIR_STORAGE_FILE_SHFS_NFS) != 1) + return ret; + else + return 0; I had to scratch my head on this one. It might be easier to read as:
if (ret< 0&& virStorageFileIsSharedFSType(path, VIR_STORAGE_FILE_SHFS_NFS) == 1) return 0; return ret;
ACK, with that tweak.
Heh. The power of cut-paste - I had pasted the call to virStorageFileIsSharedFSType() from another use further down the file, and it used != 1. Since I was already thinking in that mode, it easily made sense to me, but coming from the outside it does seem simpler your way. I pushed after making your change, as well as changing both occurences of "return 0" to "ret = 0" so that the function now has only a single exit point.
participants (2)
-
Eric Blake
-
Laine Stump