[PATCH v2] security: don't fail if built without attr support

If built without attr support removing any image will trigger qemuBlockRemoveImageMetadata (the one that emits the warning) -> qemuSecurityMoveImageMetadata -> virSecurityManagerMoveImageMetadata -> virSecurityDACMoveImageMetadata -> virSecurityDACMoveImageMetadataHelper -> virProcessRunInFork (spawns subprocess) -> virSecurityMoveRememberedLabel In there due to !HAVE_LIBATTR virFileGetXAttrQuiet will return ENOSYS and from there the chain will error out. That is wrong and looks like: libvirtd[6320]: internal error: child reported (status=125): libvirtd[6320]: Unable to remove disk metadata on vm testguest from /var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda) This change makes virSecurityDACMoveImageMetadataHelper and virSecuritySELinuxMoveImageMetadataHelper accept that error code gracefully and in that sense it is an extension of: 5214b2f1a3f "security: Don't skip label restore on file systems lacking XATTRs" which does the same for other call chains into the virFile*XAttr functions. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 6 ++++++ src/security/security_selinux.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bdc2d7edf3..7b95a6f86d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1117,6 +1117,12 @@ virSecurityDACMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED, ret = virSecurityMoveRememberedLabel(SECURITY_DAC_NAME, data->src, data->dst); virSecurityManagerMetadataUnlock(data->mgr, &state); + + if (ret == -2) { + /* Libvirt built without XATTRS */ + ret = 0; + } + return ret; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a929debe1..7bb7c2b7b1 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1975,6 +1975,12 @@ virSecuritySELinuxMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED, ret = virSecurityMoveRememberedLabel(SECURITY_SELINUX_NAME, data->src, data->dst); virSecurityManagerMetadataUnlock(data->mgr, &state); + + if (ret == -2) { + /* Libvirt built without XATTRS */ + ret = 0; + } + return ret; } -- 2.26.0

On Tue, May 26, 2020 at 3:23 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote:
If built without attr support removing any image will trigger qemuBlockRemoveImageMetadata (the one that emits the warning) -> qemuSecurityMoveImageMetadata -> virSecurityManagerMoveImageMetadata -> virSecurityDACMoveImageMetadata -> virSecurityDACMoveImageMetadataHelper -> virProcessRunInFork (spawns subprocess) -> virSecurityMoveRememberedLabel
In there due to !HAVE_LIBATTR virFileGetXAttrQuiet will return ENOSYS and from there the chain will error out.
That is wrong and looks like: libvirtd[6320]: internal error: child reported (status=125): libvirtd[6320]: Unable to remove disk metadata on vm testguest from /var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda)
This change makes virSecurityDACMoveImageMetadataHelper and virSecuritySELinuxMoveImageMetadataHelper accept that error code gracefully and in that sense it is an extension of: 5214b2f1a3f "security: Don't skip label restore on file systems lacking XATTRs" which does the same for other call chains into the virFile*XAttr functions.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was no further feedback, no build bot complains and my tests worked. Pushed the commit with your Reviewed-by added, thanks for the discussion Michal
--- src/security/security_dac.c | 6 ++++++ src/security/security_selinux.c | 6 ++++++ 2 files changed, 12 insertions(+)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bdc2d7edf3..7b95a6f86d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1117,6 +1117,12 @@ virSecurityDACMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
ret = virSecurityMoveRememberedLabel(SECURITY_DAC_NAME, data->src, data->dst); virSecurityManagerMetadataUnlock(data->mgr, &state); + + if (ret == -2) { + /* Libvirt built without XATTRS */ + ret = 0; + } + return ret; }
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a929debe1..7bb7c2b7b1 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1975,6 +1975,12 @@ virSecuritySELinuxMoveImageMetadataHelper(pid_t pid G_GNUC_UNUSED,
ret = virSecurityMoveRememberedLabel(SECURITY_SELINUX_NAME, data->src, data->dst); virSecurityManagerMetadataUnlock(data->mgr, &state); + + if (ret == -2) { + /* Libvirt built without XATTRS */ + ret = 0; + } + return ret; }
-- 2.26.0
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
participants (1)
-
Christian Ehrhardt