[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