[libvirt] [PATCH 2/2] security: SELinux: fix the transaction model's list append

Michal Privoznik mprivozn at redhat.com
Tue Jan 17 14:17:05 UTC 2017


On 01/17/2017 02:56 PM, Erik Skultety wrote:
> The problem is in the way how the list item is created prior to
> appending it to the transaction list - the @path argument is just a
> shallow copy instead of deep copy of the hostdev device's path.
> Unfortunately, the hostdev devices from which the @path is extracted, in
> order to add them into the transaction list, are only temporary and
> freed before the buildup of the qemu namespace, thus making the @path
> attribute in the transaction list NULL, causing 'permission denied' or
> 'double free' or 'unknown cause' errors.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1413773
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/security/security_selinux.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index f229b51..c799056 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -81,7 +81,7 @@ struct _virSecuritySELinuxCallbackData {
>  typedef struct _virSecuritySELinuxContextItem virSecuritySELinuxContextItem;
>  typedef virSecuritySELinuxContextItem *virSecuritySELinuxContextItemPtr;
>  struct _virSecuritySELinuxContextItem {
> -    const char *path;
> +    char *path;
>      const char *tcon;
>      bool optional;
>  };
> @@ -111,21 +111,31 @@ virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list,
>                                      const char *tcon,
>                                      bool optional)
>  {
> -    virSecuritySELinuxContextItemPtr item;
> +    int ret = -1;
> +    char *tmp = NULL;
> +    virSecuritySELinuxContextItemPtr item = NULL;
>  
>      if (VIR_ALLOC(item) < 0)
>          return -1;
>  
> -    item->path = path;
> +    if (VIR_STRDUP(tmp, path) < 0)
> +        goto cleanup;
> +
> +    item->path = tmp;
>      item->tcon = tcon;

Unfortunately, while this was enough in the DAC driver, it is not enough
here. @tcon may be dynamically allocated just for this call:

virSecuritySELinuxRestoreFileLabel ->
virSecuritySELinuxSetFilecon ->
virSecuritySELinuxSetFileconHelper ->
virSecuritySELinuxTransactionAppend ->
virSecuritySELinuxContextListAppend

However, I guess fixing that is trivial. ACK if you do so and safe for
the freeze.

Michal




More information about the libvir-list mailing list