* [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
@ 2015-09-25 4:50 Namsun Ch'o
2015-09-29 15:22 ` Eduardo Otubo
0 siblings, 1 reply; 6+ messages in thread
From: Namsun Ch'o @ 2015-09-25 4:50 UTC (permalink / raw)
To: eduardo.otubo; +Cc: pmoore, qemu-devel
Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.
Changes so far:
v1
- Created argument filters for the madvise, shmget, and shmctl syscalls.
v1 -> v2
- Added 5 new madvise flags which were present in the source code but not in
the strace which I generated.
- Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
present in GTK2, which QEMU uses but does not call directly.
v2 -> v3
- Replaced include asm/mman-common.h with sys/mman.h which is more proper.
- Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
- Removed the comma on the last entry of the madvise_flags array.
- Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.
Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
---
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f9de0d3..a353ef9 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -14,6 +14,8 @@
*/
#include <stdio.h>
#include <seccomp.h>
+#include <linux/ipc.h>
+#include <sys/mman.h>
#include "sysemu/seccomp.h"
struct QemuSeccompSyscall {
@@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
{ SCMP_SYS(rt_sigreturn), 245 },
{ SCMP_SYS(sync), 245 },
{ SCMP_SYS(pread64), 245 },
- { SCMP_SYS(madvise), 245 },
{ SCMP_SYS(set_robust_list), 245 },
{ SCMP_SYS(lseek), 245 },
{ SCMP_SYS(pselect6), 245 },
@@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
{ SCMP_SYS(arch_prctl), 240 },
{ SCMP_SYS(mkdir), 240 },
{ SCMP_SYS(fchmod), 240 },
- { SCMP_SYS(shmget), 240 },
{ SCMP_SYS(shmat), 240 },
{ SCMP_SYS(shmdt), 240 },
{ SCMP_SYS(timerfd_create), 240 },
- { SCMP_SYS(shmctl), 240 },
{ SCMP_SYS(mlockall), 240 },
{ SCMP_SYS(mlock), 240 },
{ SCMP_SYS(munlock), 240 },
@@ -264,6 +263,59 @@ int seccomp_start(void)
}
}
+ /* madvise */
+ static const int madvise_flags[] = {
+ MADV_DODUMP,
+ MADV_DONTDUMP,
+ MADV_UNMERGEABLE,
+ MADV_WILLNEED,
+ MADV_DONTFORK,
+ MADV_DONTNEED,
+ MADV_HUGEPAGE,
+ MADV_MERGEABLE
+ };
+ for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
+ rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
+ SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ }
+ rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+
+ /* shmget */
+ rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+ SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+ SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+ SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+ SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+
+ /* shmctl */
+ rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
+ SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
+ SCMP_A2(SCMP_CMP_EQ, 0));
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+ rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
+ if (rc < 0) {
+ goto seccomp_return;
+ }
+
rc = seccomp_load(ctx);
seccomp_return:
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
2015-09-25 4:50 Namsun Ch'o
@ 2015-09-29 15:22 ` Eduardo Otubo
2015-10-02 14:14 ` Eduardo Otubo
0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Otubo @ 2015-09-29 15:22 UTC (permalink / raw)
To: Namsun Ch'o; +Cc: pmoore, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4131 bytes --]
On Fri, Sep 25, 2015 at 12=50=36AM -0400, Namsun Ch'o wrote:
> Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.
>
> Changes so far:
> v1
> - Created argument filters for the madvise, shmget, and shmctl syscalls.
> v1 -> v2
> - Added 5 new madvise flags which were present in the source code but not in
> the strace which I generated.
> - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
> present in GTK2, which QEMU uses but does not call directly.
> v2 -> v3
> - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
> - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
> - Removed the comma on the last entry of the madvise_flags array.
> - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.
>
> Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
> ---
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index f9de0d3..a353ef9 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -14,6 +14,8 @@
> */
> #include <stdio.h>
> #include <seccomp.h>
> +#include <linux/ipc.h>
> +#include <sys/mman.h>
> #include "sysemu/seccomp.h"
>
> struct QemuSeccompSyscall {
> @@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> { SCMP_SYS(rt_sigreturn), 245 },
> { SCMP_SYS(sync), 245 },
> { SCMP_SYS(pread64), 245 },
> - { SCMP_SYS(madvise), 245 },
> { SCMP_SYS(set_robust_list), 245 },
> { SCMP_SYS(lseek), 245 },
> { SCMP_SYS(pselect6), 245 },
> @@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> { SCMP_SYS(arch_prctl), 240 },
> { SCMP_SYS(mkdir), 240 },
> { SCMP_SYS(fchmod), 240 },
> - { SCMP_SYS(shmget), 240 },
> { SCMP_SYS(shmat), 240 },
> { SCMP_SYS(shmdt), 240 },
> { SCMP_SYS(timerfd_create), 240 },
> - { SCMP_SYS(shmctl), 240 },
> { SCMP_SYS(mlockall), 240 },
> { SCMP_SYS(mlock), 240 },
> { SCMP_SYS(munlock), 240 },
> @@ -264,6 +263,59 @@ int seccomp_start(void)
> }
> }
>
> + /* madvise */
> + static const int madvise_flags[] = {
> + MADV_DODUMP,
> + MADV_DONTDUMP,
> + MADV_UNMERGEABLE,
> + MADV_WILLNEED,
> + MADV_DONTFORK,
> + MADV_DONTNEED,
> + MADV_HUGEPAGE,
> + MADV_MERGEABLE
> + };
> + for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
> + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
> + SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + }
> + rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> +
> + /* shmget */
> + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> + SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> + SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> +
> + /* shmctl */
> + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
> + SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
> + SCMP_A2(SCMP_CMP_EQ, 0));
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> +
> rc = seccomp_load(ctx);
>
> seccomp_return:
This looks good now.
Thanks for your contribution.
Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
ps.: I'll create a pull request with all changes made so far on Friday.
--
Eduardo Otubo
ProfitBricks GmbH
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
@ 2015-09-30 8:40 Namsun Ch'o
2015-09-30 8:45 ` Daniel P. Berrange
0 siblings, 1 reply; 6+ messages in thread
From: Namsun Ch'o @ 2015-09-30 8:40 UTC (permalink / raw)
To: eduardo.otubo; +Cc: pmoore, qemu-devel
> This looks good now.
> Thanks for your contribution.
> Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> ps.: I'll create a pull request with all changes made so far on Friday.
I was told on IRC to submit patches in smaller chunks, with a few new filters
at a time. Should I wait until it is merged, or should I go ahead and post a
v1 patch in a new thread against the patched qemu-seccomp.c now?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
2015-09-30 8:40 [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox Namsun Ch'o
@ 2015-09-30 8:45 ` Daniel P. Berrange
0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2015-09-30 8:45 UTC (permalink / raw)
To: Namsun Ch'o; +Cc: pmoore, qemu-devel, eduardo.otubo
On Wed, Sep 30, 2015 at 04:40:42AM -0400, Namsun Ch'o wrote:
> > This looks good now.
> > Thanks for your contribution.
>
> > Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
>
> > ps.: I'll create a pull request with all changes made so far on Friday.
>
> I was told on IRC to submit patches in smaller chunks, with a few new filters
> at a time. Should I wait until it is merged, or should I go ahead and post a
> v1 patch in a new thread against the patched qemu-seccomp.c now?
There's no need to wait for things to be merged - feel free to post further
patches based on the patch you already submitted. Just mention when posting
them that they're a patch against your previous posting.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
2015-09-29 15:22 ` Eduardo Otubo
@ 2015-10-02 14:14 ` Eduardo Otubo
0 siblings, 0 replies; 6+ messages in thread
From: Eduardo Otubo @ 2015-10-02 14:14 UTC (permalink / raw)
To: Namsun Ch'o, berrange, qemu-devel, pmoore
[-- Attachment #1: Type: text/plain, Size: 4687 bytes --]
On Tue, Sep 29, 2015 at 05=22=44PM +0200, Eduardo Otubo wrote:
> On Fri, Sep 25, 2015 at 12=50=36AM -0400, Namsun Ch'o wrote:
> > Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.
> >
> > Changes so far:
> > v1
> > - Created argument filters for the madvise, shmget, and shmctl syscalls.
> > v1 -> v2
> > - Added 5 new madvise flags which were present in the source code but not in
> > the strace which I generated.
> > - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
> > present in GTK2, which QEMU uses but does not call directly.
> > v2 -> v3
> > - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
> > - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
> > - Removed the comma on the last entry of the madvise_flags array.
> > - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.
> >
> > Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
> > ---
> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> > index f9de0d3..a353ef9 100644
> > --- a/qemu-seccomp.c
> > +++ b/qemu-seccomp.c
> > @@ -14,6 +14,8 @@
> > */
> > #include <stdio.h>
> > #include <seccomp.h>
> > +#include <linux/ipc.h>
> > +#include <sys/mman.h>
> > #include "sysemu/seccomp.h"
> >
> > struct QemuSeccompSyscall {
> > @@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> > { SCMP_SYS(rt_sigreturn), 245 },
> > { SCMP_SYS(sync), 245 },
> > { SCMP_SYS(pread64), 245 },
> > - { SCMP_SYS(madvise), 245 },
> > { SCMP_SYS(set_robust_list), 245 },
> > { SCMP_SYS(lseek), 245 },
> > { SCMP_SYS(pselect6), 245 },
> > @@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> > { SCMP_SYS(arch_prctl), 240 },
> > { SCMP_SYS(mkdir), 240 },
> > { SCMP_SYS(fchmod), 240 },
> > - { SCMP_SYS(shmget), 240 },
> > { SCMP_SYS(shmat), 240 },
> > { SCMP_SYS(shmdt), 240 },
> > { SCMP_SYS(timerfd_create), 240 },
> > - { SCMP_SYS(shmctl), 240 },
> > { SCMP_SYS(mlockall), 240 },
> > { SCMP_SYS(mlock), 240 },
> > { SCMP_SYS(munlock), 240 },
> > @@ -264,6 +263,59 @@ int seccomp_start(void)
> > }
> > }
> >
> > + /* madvise */
> > + static const int madvise_flags[] = {
> > + MADV_DODUMP,
> > + MADV_DONTDUMP,
> > + MADV_UNMERGEABLE,
> > + MADV_WILLNEED,
> > + MADV_DONTFORK,
> > + MADV_DONTNEED,
> > + MADV_HUGEPAGE,
> > + MADV_MERGEABLE
> > + };
> > + for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
> > + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
> > + SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > + }
> > + rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > +
> > + /* shmget */
> > + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> > + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> > + SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> > + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> > + SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > + rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > +
> > + /* shmctl */
> > + rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
> > + SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
> > + SCMP_A2(SCMP_CMP_EQ, 0));
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > + rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
> > + if (rc < 0) {
> > + goto seccomp_return;
> > + }
> > +
> > rc = seccomp_load(ctx);
> >
> > seccomp_return:
>
> This looks good now.
> Thanks for your contribution.
>
> Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
>
> ps.: I'll create a pull request with all changes made so far on Friday.
>
The pull request will be delayed a little bit due to some new patches
incoming. Let's just set an agreement on how to approach regarding the
"-runas and -chroot" patch and will prepare just a single batch for pull
reuqest to Peter.
Regards,
--
Eduardo Otubo
ProfitBricks GmbH
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox
@ 2015-10-04 4:15 Namsun Ch'o
0 siblings, 0 replies; 6+ messages in thread
From: Namsun Ch'o @ 2015-10-04 4:15 UTC (permalink / raw)
To: eduardo.otubo; +Cc: pmoore, qemu-devel
> The pull request will be delayed a little bit due to some new patches
> incoming. Let's just set an agreement on how to approach regarding the
> "-runas and -chroot" patch and will prepare just a single batch for pull
> reuqest to Peter.
I also noticed that I had made that patch over the chroot and setuid patch, so
it won't apply without patch's fuzzing enabled. So that's probably a good
idea.
If I have a patch of 436 lines (296 added, 25 removed) which adds argument
filters for 12 syscalls, should I submit it in a new thread as a whole, or
break it up into like 3 syscalls at a time?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-04 4:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-30 8:40 [Qemu-devel] [PATCH v3] Add argument filters to the seccomp sandbox Namsun Ch'o
2015-09-30 8:45 ` Daniel P. Berrange
-- strict thread matches above, loose matches on Subject: below --
2015-10-04 4:15 Namsun Ch'o
2015-09-25 4:50 Namsun Ch'o
2015-09-29 15:22 ` Eduardo Otubo
2015-10-02 14:14 ` Eduardo Otubo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).