[PATCH] qemu_slirp: Check if helper exists before fetching its capabilities

[Daniel P. Berrangé]

On Tue, Oct 13, 2020 at 03:32:28PM +0200, Michal Privoznik wrote:
> I've noticed when running libvirtd in the session mode that
> whenever I start a virtual machine the following error is printed
> into logs:
> 
>   error : cannot execute binary /usr/bin/slirp-helper: No such file or directory
> 
> The error message is produced in qemuSlirpNewForHelper() which
> does attempt to be a NO-OP if the helper doesn't exist by
> checking if its path is NULL, but that's not usually the case
> because in the default config (in virQEMUDriverConfigNew()) its
> path is initialized to QEMU_SLIRP_HELPER. And while it is true
> that during configure we try to get the actual path of the helper
> we fallback to the path above if not found.
> 
> Fix the check so that the function checks whether the helper
> exists and is executable.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_slirp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> index d2e4ed79be..4e5ab12727 100644
> --- a/src/qemu/qemu_slirp.c
> +++ b/src/qemu/qemu_slirp.c
> @@ -101,7 +101,8 @@ qemuSlirpNewForHelper(const char *helper)
>      virJSONValuePtr featuresJSON;
>      size_t i, nfeatures;
>  
> -    if (!helper)
> +    if (!helper ||
> +        !virFileIsExecutable(helper))
>          return NULL;

This API has way bigger problems than this. It is reporting errors using
virReportError, but the callers ignore them and can't distinguish real
errors from expected errors.

I'm working on a more comprehensive fix and will send patch shortly


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|