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

Michal Privoznik posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/80c86614e3b21dda074b81fe9c3866fdbbb7420e.1666769319.git.mprivozn@redhat.com
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>
meson.build            |  9 +++++++++
softmmu/qemu-seccomp.c | 13 +++++++++++++
2 files changed, 22 insertions(+)
[PATCH v2] seccomp: Get actual errno value from failed seccomp functions
Posted by Michal Privoznik 1 year, 6 months ago
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---

v2 of:

https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg04509.html

diff to v1:
- added comment when setting SYSRAWRC attribute per Philippe's
  suggestion

 meson.build            |  9 +++++++++
 softmmu/qemu-seccomp.c | 13 +++++++++++++
 2 files changed, 22 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..d66a2a1226 100644
--- a/softmmu/qemu-seccomp.c
+++ b/softmmu/qemu-seccomp.c
@@ -312,6 +312,19 @@ static int seccomp_start(uint32_t seccomp_opts, Error **errp)
         goto seccomp_return;
     }
 
+#if defined(CONFIG_SECCOMP_SYSRAWRC)
+    /*
+     * This must be the first seccomp_attr_set() call to have full
+     * error propagation from subsequent seccomp APIs.
+     */
+    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,
-- 
2.37.4


Re: [PATCH v2] seccomp: Get actual errno value from failed seccomp functions
Posted by Daniel P. Berrangé 1 year, 6 months ago
On Wed, Oct 26, 2022 at 09:30:24AM +0200, 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> 
> v2 of:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg04509.html
> 
> diff to v1:
> - added comment when setting SYSRAWRC attribute per Philippe's
>   suggestion
> 
>  meson.build            |  9 +++++++++
>  softmmu/qemu-seccomp.c | 13 +++++++++++++
>  2 files changed, 22 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With 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 :|


Re: [PATCH v2] seccomp: Get actual errno value from failed seccomp functions
Posted by Michal Prívozník 1 year, 6 months ago
On 10/26/22 14:33, Daniel P. Berrangé wrote:
> On Wed, Oct 26, 2022 at 09:30:24AM +0200, 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>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>
>> v2 of:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg04509.html
>>
>> diff to v1:
>> - added comment when setting SYSRAWRC attribute per Philippe's
>>   suggestion
>>
>>  meson.build            |  9 +++++++++
>>  softmmu/qemu-seccomp.c | 13 +++++++++++++
>>  2 files changed, 22 insertions(+)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thank you. Can somebody merge this please? I don't have commit access.

Michal


Re: [PATCH v2] seccomp: Get actual errno value from failed seccomp functions
Posted by Daniel P. Berrangé 1 year, 6 months ago
On Thu, Oct 27, 2022 at 02:55:02PM +0200, Michal Prívozník wrote:
> On 10/26/22 14:33, Daniel P. Berrangé wrote:
> > On Wed, Oct 26, 2022 at 09:30:24AM +0200, 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>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >>
> >> v2 of:
> >>
> >> https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg04509.html
> >>
> >> diff to v1:
> >> - added comment when setting SYSRAWRC attribute per Philippe's
> >>   suggestion
> >>
> >>  meson.build            |  9 +++++++++
> >>  softmmu/qemu-seccomp.c | 13 +++++++++++++
> >>  2 files changed, 22 insertions(+)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Thank you. Can somebody merge this please? I don't have commit access.

I'm seccomp subsystem maintainer and have it queued.

With 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 :|