[libvirt] [PATCH] storage: skip selinux cleanup when fd not available

[Osier Yang]

On 27/11/13 12:00, Eric Blake wrote:
> When attempting to backport gluster pools to an older version
> where there is no VIR_STRDUP, I got a crash from calling
> strdup(,NULL).  Rather than relying on the current else branch
> safely doing nothing when there is no fd, it is easier to just
> skip it. While at it, there's no need to explicitly set
> perms.label to NULL after a VIR_FREE().
>
> * src/storage/storage_backend.c
> (virStorageBackendUpdateVolTargetInfoFD): Minor optimization.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>
>   src/storage/storage_backend.c | 26 ++++++++++++--------------
>   1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index bde39d6..b08d646 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -1383,28 +1383,26 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target,
>
>       VIR_FREE(target->perms.label);
>
>   #if WITH_SELINUX
>       /* XXX: make this a security driver call */
> -    if (fd >= 0 && fgetfilecon_raw(fd, &filecon) == -1) {
> -        if (errno != ENODATA && errno != ENOTSUP) {
> -            virReportSystemError(errno,
> -                                 _("cannot get file context of '%s'"),
> -                                 target->path);
> -            return -1;
> +    if (fd >= 0) {
> +        if (fgetfilecon_raw(fd, &filecon) == -1) {
> +            if (errno != ENODATA && errno != ENOTSUP) {
> +                virReportSystemError(errno,
> +                                     _("cannot get file context of '%s'"),
> +                                     target->path);
> +                return -1;
> +            }
>           } else {
> -            target->perms.label = NULL;
> -        }
> -    } else {
> -        if (VIR_STRDUP(target->perms.label, filecon) < 0) {
> +            if (VIR_STRDUP(target->perms.label, filecon) < 0) {
> +                freecon(filecon);
> +                return -1;
> +            }
>               freecon(filecon);
> -            return -1;
>           }
> -        freecon(filecon);
>       }
> -#else
> -    target->perms.label = NULL;
>   #endif
>
>       return 0;
>   }
>

ACK