[libvirt] [PATCH] util: Don't overflow on errno in virFileAccessibleAs

[Eric Blake]

On 03/08/2012 03:37 AM, Michal Privoznik wrote:
> If we need to virFork() to check assess() under different
> UID+GID we need to translate returned status via WEXITSTATUS().
> Otherwise, we may return values greater than 255 which is
> obviously wrong.
> ---
>  src/util/util.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/src/util/util.c b/src/util/util.c
> index 548ed1c..15e6cfa 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -724,8 +724,13 @@ virFileAccessibleAs(const char *path, int mode,
>                  return -1;
>          }
>  
> +        if (!WIFEXITED(status)) {
> +            errno = EINTR;
> +            return -1;
> +        }

ACK; this matches what we do in virFileOpenForked.

However, I still see two lingering issues that might be worth revisiting:

1. I wonder if virWaitPid() would be easier to use if it only returned
success on WIFEXITED, and set *status to WEXITSTAUS(), while returning
-1 on any child dying due to a signal.  I'd have to audit the users of
virWaitPid to see if they can all be simplified by this change, or if
there really is a user that needs to know if a child exited due to a signal.

2. This still shares the latent bug in virFileOpenForked that errno is
not always guaranteed to be less than 255; on GNU Hurd, this code is
broken - but libvirt doesn't compile on Hurd.  A true fix would be to
enumerate specific errno values to specific exit codes, and map all
others to a catch-all; see how daemon/libvirtd.c has virDaemonErr for
this purpose.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120308/c99700a0/attachment-0001.sig>