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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 26, 2020 at 3:23 PM Christian Ehrhardt
<christian.ehrhardt@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux