[Qemu-devel] [PULL 0/2] seccomp branch queue

[Qemu-devel] [PULL 0/2] seccomp branch queue

[Eduardo Otubo]

Hi,

This is the seccomp branch queue with fixes regarding a build crach on armv7l
and adding new syscalls to the whitelist. Details below.

The following changes since commit 6e76d125f244e10676b917208f2a074729820246:

  Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)

are available in the git repository at:

  git://github.com/otubo/qemu.git tags/pull-seccomp-20141110

for you to fetch changes up to f5c76d3eb66a63604d9d8b47ce94268f9babea10:

  seccomp: change configure to avoid arm 32 to break (2014-11-07 16:42:18 +0100)

----------------------------------------------------------------
seccomp: change configure to avoid arm 32 to break
seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()

----------------------------------------------------------------
Eduardo Otubo (1):
      seccomp: change configure to avoid arm 32 to break

Philipp Gesang (1):
      seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()

 configure      | 3 ++-
 qemu-seccomp.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
1.9.1

[Qemu-devel] [PULL 1/2] seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()

[Eduardo Otubo]

From: Philipp Gesang <philipp.gesang@intra2net.com>

fallocate() is needed for snapshotting. If it isn’t whitelisted

    $ qemu-img create -f qcow2 x.qcow 1G
    Formatting 'x.qcow', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off
    $ qemu-kvm -display none -monitor stdio -sandbox on x.qcow
    QEMU 2.1.50 monitor - type 'help' for more information
    (qemu) savevm foo
    (qemu) loadvm foo

will fail, as will subsequent savevm commands on the same image.

fadvise64(), inotify_init1(), inotify_add_watch() are needed by
the SDL display. Without the whitelist entries,

    qemu-kvm -sandbox on

fails immediately.

In my tests fadvise64() is called 50--51 times per VM run. That
number seems independent of the duration of the run. fallocate(),
inotify_init1(), inotify_add_watch() are called once each.
Accordingly, they are added to the whitelist at a very low
priority.

Signed-off-by: Philipp Gesang <philipp.gesang@intra2net.com>
---
 qemu-seccomp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 0503764..af6a375 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -231,7 +231,11 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(shmctl), 240 },
     { SCMP_SYS(mlock), 240 },
     { SCMP_SYS(munlock), 240 },
-    { SCMP_SYS(semctl), 240 }
+    { SCMP_SYS(semctl), 240 },
+    { SCMP_SYS(fallocate), 240 },
+    { SCMP_SYS(fadvise64), 240 },
+    { SCMP_SYS(inotify_init1), 240 },
+    { SCMP_SYS(inotify_add_watch), 240 }
 };
 
 int seccomp_start(void)
-- 
1.9.1

[Qemu-devel] [PULL 2/2] seccomp: change configure to avoid arm 32 to break

[Eduardo Otubo]

Current stable version of libseccomp (2.1.1) only supports i386 and
x86_64 archs correctly. This patch limits the usage of the syscall
filter for those archs and updates to the correct last version of
libseccomp.

This patch also fixes the bug:
https://bugs.launchpad.net/qemu/+bug/1363641

Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 2f17bf3..47048f0 100755
--- a/configure
+++ b/configure
@@ -1823,7 +1823,8 @@ fi
 # libseccomp check
 
 if test "$seccomp" != "no" ; then
-    if $pkg_config --atleast-version=2.1.0 libseccomp; then
+    if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
+        $pkg_config --atleast-version=2.1.1 libseccomp; then
         libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
         QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
 	seccomp="yes"
-- 
1.9.1

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Peter Maydell]

On 10 November 2014 14:21, Eduardo Otubo <eduardo.otubo@profitbricks.com> wrote:
> Hi,
>
> This is the seccomp branch queue with fixes regarding a build crach on armv7l
> and adding new syscalls to the whitelist. Details below.
>
> The following changes since commit 6e76d125f244e10676b917208f2a074729820246:
>
>   Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)
>
> are available in the git repository at:
>
>   git://github.com/otubo/qemu.git tags/pull-seccomp-20141110
>
> for you to fetch changes up to f5c76d3eb66a63604d9d8b47ce94268f9babea10:
>
>   seccomp: change configure to avoid arm 32 to break (2014-11-07 16:42:18 +0100)
>
> ----------------------------------------------------------------
> seccomp: change configure to avoid arm 32 to break
> seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()
>
> ----------------------------------------------------------------
> Eduardo Otubo (1):
>       seccomp: change configure to avoid arm 32 to break
>
> Philipp Gesang (1):
>       seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()

Hi. I'm afraid I'm going to have to ask you to respin this,
because the patches are missing various reviewed-by/signed-off etc
tags.

Firstly, and least critically, the configure patch should have
the reviewed-by and acked-by tags that people have posted on
the mailing list for it; it's useful to keep a permanent record
of these in the commit history (for instance it can give a good
idea of who to cc if a change turns out to have a bug in future).

More significantly, the patch from Philipp is missing a
Signed-off-by: line from you. Every commit in a pullreq should
have a signed-off-by: from the submaintainer(s) of the tree
that's being pulled, because it represents your assertion that
these changes are good to go into QEMU without further checking.

Sorry to have to be picky about this. The good news is, your
signed tag seems OK. You should make sure you've pushed your
public key out to the public keyservers if you haven't already
(gpg --send-keys 12F8BD2F) since it's not currently visible
there [there is a lag of a day or two though so if you've
sent it out recently it's probably just in that delay.]

thanks
-- PMM

[Qemu-devel] [PULL 0/2] seccomp branch queue

[Eduardo Otubo]

Hi,

This is the seccomp branch queue with fixes regarding a build crach on armv7l
and adding new syscalls to the whitelist. Details below.

The following changes since commit 6e76d125f244e10676b917208f2a074729820246:

  Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)

are available in the git repository at:

  git://github.com/otubo/qemu.git tags/pull-seccomp-20141111

for you to fetch changes up to 4cc47f8b3cc4f32586ba2f7fce1dc267da774a69:

  seccomp: change configure to avoid arm 32 to break (2014-11-11 17:05:21 +0100)

----------------------------------------------------------------
seccomp: change configure to avoid arm 32 to break
seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()
----------------------------------------------------------------
Eduardo Otubo (1):
      seccomp: change configure to avoid arm 32 to break

Philipp Gesang (1):
      seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()

 configure      | 3 ++-
 qemu-seccomp.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

Eduardo Otubo (1):
  seccomp: change configure to avoid arm 32 to break

Philipp Gesang (1):
  seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1()
    and inotify_add_watch()

 configure      | 3 ++-
 qemu-seccomp.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
1.9.1

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Eduardo Otubo]

On Mon, Nov 10, 2014 at 08:50:07PM +0000, Peter Maydell wrote:
> On 10 November 2014 14:21, Eduardo Otubo <eduardo.otubo@profitbricks.com> wrote:
> > Hi,
> >
> > This is the seccomp branch queue with fixes regarding a build crach on armv7l
> > and adding new syscalls to the whitelist. Details below.
> >
> > The following changes since commit 6e76d125f244e10676b917208f2a074729820246:
> >
> >   Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)
> >
> > are available in the git repository at:
> >
> >   git://github.com/otubo/qemu.git tags/pull-seccomp-20141110
> >
> > for you to fetch changes up to f5c76d3eb66a63604d9d8b47ce94268f9babea10:
> >
> >   seccomp: change configure to avoid arm 32 to break (2014-11-07 16:42:18 +0100)
> >
> > ----------------------------------------------------------------
> > seccomp: change configure to avoid arm 32 to break
> > seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()
> >
> > ----------------------------------------------------------------
> > Eduardo Otubo (1):
> >       seccomp: change configure to avoid arm 32 to break
> >
> > Philipp Gesang (1):
> >       seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()
> 
> Hi. I'm afraid I'm going to have to ask you to respin this,
> because the patches are missing various reviewed-by/signed-off etc
> tags.
> 
> Firstly, and least critically, the configure patch should have
> the reviewed-by and acked-by tags that people have posted on
> the mailing list for it; it's useful to keep a permanent record
> of these in the commit history (for instance it can give a good
> idea of who to cc if a change turns out to have a bug in future).
> 
> More significantly, the patch from Philipp is missing a
> Signed-off-by: line from you. Every commit in a pullreq should
> have a signed-off-by: from the submaintainer(s) of the tree
> that's being pulled, because it represents your assertion that
> these changes are good to go into QEMU without further checking.
> 

My apologies! I was so worried about the key and tagging being set
properly on my branch that those lines completely passed from my review.
I resent the pull request with everything double-checked. I hope
everything is 0K now.

> Sorry to have to be picky about this. The good news is, your
> signed tag seems OK. You should make sure you've pushed your
> public key out to the public keyservers if you haven't already
> (gpg --send-keys 12F8BD2F) since it's not currently visible
> there [there is a lag of a day or two though so if you've
> sent it out recently it's probably just in that delay.]
> 

I just pushed my key to the public server. So it might take a while to
appear as valid.

Thanks for the help on this,
and sorry again for the lapses.

Regards,

-- 
Eduardo Otubo
ProfitBricks GmbH

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Peter Maydell]

On 11 November 2014 16:21, Eduardo Otubo <eduardo.otubo@profitbricks.com> wrote:
> Hi,
>
> This is the seccomp branch queue with fixes regarding a build crach on armv7l
> and adding new syscalls to the whitelist. Details below.
>
> The following changes since commit 6e76d125f244e10676b917208f2a074729820246:
>
>   Update version for v2.2.0-rc0 release (2014-11-05 15:21:04 +0000)
>
> are available in the git repository at:
>
>   git://github.com/otubo/qemu.git tags/pull-seccomp-20141111
>
> for you to fetch changes up to 4cc47f8b3cc4f32586ba2f7fce1dc267da774a69:
>
>   seccomp: change configure to avoid arm 32 to break (2014-11-11 17:05:21 +0100)
>
> ----------------------------------------------------------------
> seccomp: change configure to avoid arm 32 to break
> seccomp: whitelist syscalls fallocate(), fadvise64(), inotify_init1() and inotify_add_watch()
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 0/2] seccomp branch queue

[Eduardo Otubo]

The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc:

  Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100)

are available in the Git repository at:

  https://github.com/otubo/qemu.git tags/pull-seccomp-20180725

for you to fetch changes up to 5b2f59307372bae13a2ff95706646674eccb65e0:

  RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available (2018-07-25 16:07:31 +0200)

----------------------------------------------------------------
pull-seccomp-20180725

----------------------------------------------------------------
Marc-André Lureau (2):
      seccomp: use SIGSYS signal instead of killing the thread
      RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available

 qemu-seccomp.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

-- 
2.17.1

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Peter Maydell]

On 25 July 2018 at 15:16, Eduardo Otubo <otubo@redhat.com> wrote:
> The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc:
>
>   Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/otubo/qemu.git tags/pull-seccomp-20180725
>
> for you to fetch changes up to 5b2f59307372bae13a2ff95706646674eccb65e0:
>
>   RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available (2018-07-25 16:07:31 +0200)
>
> ----------------------------------------------------------------
> pull-seccomp-20180725
>
> ----------------------------------------------------------------
> Marc-André Lureau (2):
>       seccomp: use SIGSYS signal instead of killing the thread
>       RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available

Hi. This fails to compile with clang:

  CC      qemu-seccomp.o
qemu-seccomp.c:112:1: error: unused function 'qemu_seccomp'
[-Werror,-Wunused-function]
qemu_seccomp(unsigned int operation, unsigned int flags, void *args)
^

This is because clang is stricter about warning about static inline
functions defined in .c files but never used and your ifdef
guard on the callsite is not matched by one around the function
definition.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Eduardo Otubo]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On 26/07/2018 - 11:47:46, Peter Maydell wrote:
> On 25 July 2018 at 15:16, Eduardo Otubo <otubo@redhat.com> wrote:
> > The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc:
> >
> >   Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/otubo/qemu.git tags/pull-seccomp-20180725
> >
> > for you to fetch changes up to 5b2f59307372bae13a2ff95706646674eccb65e0:
> >
> >   RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available (2018-07-25 16:07:31 +0200)
> >
> > ----------------------------------------------------------------
> > pull-seccomp-20180725
> >
> > ----------------------------------------------------------------
> > Marc-André Lureau (2):
> >       seccomp: use SIGSYS signal instead of killing the thread
> >       RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available
> 
> Hi. This fails to compile with clang:
> 
>   CC      qemu-seccomp.o
> qemu-seccomp.c:112:1: error: unused function 'qemu_seccomp'
> [-Werror,-Wunused-function]
> qemu_seccomp(unsigned int operation, unsigned int flags, void *args)
> ^
> 
> This is because clang is stricter about warning about static inline
> functions defined in .c files but never used and your ifdef
> guard on the callsite is not matched by one around the function
> definition.
> 

Peter, sorry for not catching that before.
Marc, can you fix and resend?

-- 
Eduardo Otubo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Marc-André Lureau]

Hi

On Thu, Jul 26, 2018 at 12:47 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 15:16, Eduardo Otubo <otubo@redhat.com> wrote:
>> The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc:
>>
>>   Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/otubo/qemu.git tags/pull-seccomp-20180725
>>
>> for you to fetch changes up to 5b2f59307372bae13a2ff95706646674eccb65e0:
>>
>>   RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available (2018-07-25 16:07:31 +0200)
>>
>> ----------------------------------------------------------------
>> pull-seccomp-20180725
>>
>> ----------------------------------------------------------------
>> Marc-André Lureau (2):
>>       seccomp: use SIGSYS signal instead of killing the thread
>>       RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available
>
> Hi. This fails to compile with clang:
>
>   CC      qemu-seccomp.o
> qemu-seccomp.c:112:1: error: unused function 'qemu_seccomp'
> [-Werror,-Wunused-function]
> qemu_seccomp(unsigned int operation, unsigned int flags, void *args)
> ^
>
> This is because clang is stricter about warning about static inline
> functions defined in .c files but never used and your ifdef
> guard on the callsite is not matched by one around the function
> definition.
>

https://lkml.org/lkml/2017/6/6/631 ;)



-- 
Marc-André Lureau

Re: [Qemu-devel] [PULL 0/2] seccomp branch queue

[Marc-André Lureau]

H

On Thu, Jul 26, 2018 at 2:04 PM, Eduardo Otubo <otubo@redhat.com> wrote:
> On 26/07/2018 - 11:47:46, Peter Maydell wrote:
>> On 25 July 2018 at 15:16, Eduardo Otubo <otubo@redhat.com> wrote:
>> > The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc:
>> >
>> >   Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100)
>> >
>> > are available in the Git repository at:
>> >
>> >   https://github.com/otubo/qemu.git tags/pull-seccomp-20180725
>> >
>> > for you to fetch changes up to 5b2f59307372bae13a2ff95706646674eccb65e0:
>> >
>> >   RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available (2018-07-25 16:07:31 +0200)
>> >
>> > ----------------------------------------------------------------
>> > pull-seccomp-20180725
>> >
>> > ----------------------------------------------------------------
>> > Marc-André Lureau (2):
>> >       seccomp: use SIGSYS signal instead of killing the thread
>> >       RFC: seccomp: prefer SCMP_ACT_KILL_PROCESS if available
>>
>> Hi. This fails to compile with clang:
>>
>>   CC      qemu-seccomp.o
>> qemu-seccomp.c:112:1: error: unused function 'qemu_seccomp'
>> [-Werror,-Wunused-function]
>> qemu_seccomp(unsigned int operation, unsigned int flags, void *args)
>> ^
>>
>> This is because clang is stricter about warning about static inline
>> functions defined in .c files but never used and your ifdef
>> guard on the callsite is not matched by one around the function
>> definition.
>>
>
> Peter, sorry for not catching that before.
> Marc, can you fix and resend?

I suggest to drop that patch from 3.0. Since it will require a newer
libseccomp to be actually useful, it can be delayed imho.



-- 
Marc-André Lureau