Re: [PATCH] seccomp: Get actual errno value from failed seccomp functions

[Philippe Mathieu-Daudé]

Cc'ing Daniel (maintainer)

On 25/10/22 13:59, Michal Privoznik wrote:
> Upon failure, a libseccomp API returns actual errno value very
> rarely. Fortunately, after its commit 34bf78ab (contained in
> 2.5.0 release), the SCMP_FLTATR_API_SYSRAWRC attribute can be set
> which makes subsequent APIs return true errno on failure.
>
> This is especially critical when seccomp_load() fails, because
> generic -ECANCELED says nothing.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   meson.build            | 9 +++++++++
>   softmmu/qemu-seccomp.c | 9 +++++++++
>   2 files changed, 18 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index b686dfef75..5f114c89d9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -636,10 +636,16 @@ if vmnet.found() and not 
> cc.has_header_symbol('vmnet/vmnet.h',
>   endif
>   
>   seccomp = not_found
> +seccomp_has_sysrawrc = false
>   if not get_option('seccomp').auto() or have_system or have_tools
>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>                          required: get_option('seccomp'),
>                          method: 'pkg-config', kwargs: static_kwargs)
> +  if seccomp.found()
> +    seccomp_has_sysrawrc = cc.has_header_symbol('seccomp.h',
> +                                                'SCMP_FLTATR_API_SYSRAWRC',
> +                                                dependencies: seccomp)
> +  endif
>   endif
>   
>   libcap_ng = not_found
> @@ -1849,6 +1855,9 @@ config_host_data.set('CONFIG_RDMA', rdma.found())
>   config_host_data.set('CONFIG_SDL', sdl.found())
>   config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
>   config_host_data.set('CONFIG_SECCOMP', seccomp.found())
> +if seccomp.found()
> +  config_host_data.set('CONFIG_SECCOMP_SYSRAWRC', seccomp_has_sysrawrc)
> +endif
>   config_host_data.set('CONFIG_SNAPPY', snappy.found())
>   config_host_data.set('CONFIG_TPM', have_tpm)
>   config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
> diff --git a/softmmu/qemu-seccomp.c b/softmmu/qemu-seccomp.c
> index deaf8a4ef5..05abf257c4 100644
> --- a/softmmu/qemu-seccomp.c
> +++ b/softmmu/qemu-seccomp.c
> @@ -312,6 +312,15 @@ static int seccomp_start(uint32_t seccomp_opts, Error 
> **errp)
>           goto seccomp_return;
>       }
>   
> +#if defined(CONFIG_SECCOMP_SYSRAWRC)

Maybe add some comment here such:

 /*
  * This must be the first seccomp_attr_set() call to have full
  * error propagation.
  */

> +    rc = seccomp_attr_set(ctx, SCMP_FLTATR_API_SYSRAWRC, 1);
> +    if (rc != 0) {
> +        error_setg_errno(errp, -rc,
> +                         "failed to set seccomp rawrc attribute");
> +        goto seccomp_return;
> +    }
> +#endif
> +
>       rc = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1);
>       if (rc != 0) {
>           error_setg_errno(errp, -rc,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>