* [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support @ 2019-07-10 13:06 Eric Ren 2019-07-11 11:25 ` Dr. David Alan Gilbert 2019-07-12 8:04 ` Stefan Hajnoczi 0 siblings, 2 replies; 7+ messages in thread From: Eric Ren @ 2019-07-10 13:06 UTC (permalink / raw) To: virtio-fs SCMP_FLTATTR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater. So, conditional compile to make virtio-fs work on older host kernel. Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> --- contrib/virtiofsd/seccomp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c index 4e388adc9c..5a28a90859 100644 --- a/contrib/virtiofsd/seccomp.c +++ b/contrib/virtiofsd/seccomp.c @@ -11,6 +11,7 @@ #include <errno.h> #include <seccomp.h> #include <glib.h> +#include <linux/version.h> #include "seccomp.h" static const int syscall_whitelist[] = { @@ -92,9 +93,12 @@ void setup_seccomp(void) err(1, "seccomp_init()"); } +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); } +#endif for (i = 0; i < G_N_ELEMENTS(syscall_whitelist); i++) { if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, -- 2.17.2 (Apple Git-113) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-10 13:06 [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support Eric Ren @ 2019-07-11 11:25 ` Dr. David Alan Gilbert 2019-07-11 12:29 ` Eric Ren 2019-07-12 8:04 ` Stefan Hajnoczi 1 sibling, 1 reply; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-07-11 11:25 UTC (permalink / raw) To: Eric Ren; +Cc: virtio-fs * Eric Ren (renzhen@linux.alibaba.com) wrote: > SCMP_FLTATTR_CTL_TSYNC flag is only available on > Linux Kernel 3.17 or greater. So, conditional compile > to make virtio-fs work on older host kernel. > > Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> > --- > contrib/virtiofsd/seccomp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c > index 4e388adc9c..5a28a90859 100644 > --- a/contrib/virtiofsd/seccomp.c > +++ b/contrib/virtiofsd/seccomp.c > @@ -11,6 +11,7 @@ > #include <errno.h> > #include <seccomp.h> > #include <glib.h> > +#include <linux/version.h> > #include "seccomp.h" > > static const int syscall_whitelist[] = { > @@ -92,9 +93,12 @@ void setup_seccomp(void) > err(1, "seccomp_init()"); > } > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) I don't really like using kernel versions, because sometimes downstreams backport stuff (I checked and it looks like RHEL7 did this somewhere around 7.5). If I understand correctly the right thing to do is check the SCMP_VER_MAJOR/MINOR/MICRO version defines; and I think tsync came in with 2.3.1. > + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater > if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { > err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); Also, what happens if this fails? e.g. I run it on an older kernel than it's built for; do we actually fail here or just print the error. Eithe rway, is it actually safe without this define - or does the thread which actually runs the work not get the support? Dave > } > +#endif > > for (i = 0; i < G_N_ELEMENTS(syscall_whitelist); i++) { > if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, > -- > 2.17.2 (Apple Git-113) > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-11 11:25 ` Dr. David Alan Gilbert @ 2019-07-11 12:29 ` Eric Ren 2019-07-11 12:32 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 7+ messages in thread From: Eric Ren @ 2019-07-11 12:29 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: virtio-fs On Thu, Jul 11, 2019 at 12:25:58PM +0100, Dr. David Alan Gilbert wrote: > * Eric Ren (renzhen@linux.alibaba.com) wrote: > > SCMP_FLTATTR_CTL_TSYNC flag is only available on > > Linux Kernel 3.17 or greater. So, conditional compile > > to make virtio-fs work on older host kernel. > > > > Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> > > --- > > contrib/virtiofsd/seccomp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c > > index 4e388adc9c..5a28a90859 100644 > > --- a/contrib/virtiofsd/seccomp.c > > +++ b/contrib/virtiofsd/seccomp.c > > @@ -11,6 +11,7 @@ > > #include <errno.h> > > #include <seccomp.h> > > #include <glib.h> > > +#include <linux/version.h> > > #include "seccomp.h" > > > > static const int syscall_whitelist[] = { > > @@ -92,9 +93,12 @@ void setup_seccomp(void) > > err(1, "seccomp_init()"); > > } > > > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) > > I don't really like using kernel versions, because sometimes downstreams > backport stuff (I checked and it looks like RHEL7 did this somewhere > around 7.5). > > If I understand correctly the right thing to do is check the > SCMP_VER_MAJOR/MINOR/MICRO version defines; and I think tsync came in > with 2.3.1. > > > + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater > > if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { > > err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); > > Also, what happens if this fails? e.g. I run it on an older kernel than > it's built for; do we actually fail here or just print the error. So the result also applies if checking SCMP_VER_MAJOR/MINOR/MICRO defines :-/ > > Eithe rway, is it actually safe without this define - Actually I don't know the exact effect of SCMP_FLTATR_CTL_TSYNC attr. What if we gives a warning instead of error if failing to set it? > or does the thread > which actually runs the work not get the support? Sorry, I fail to get your point here? Regards, Eric > > Dave > > > } > > +#endif > > > > for (i = 0; i < G_N_ELEMENTS(syscall_whitelist); i++) { > > if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, > > -- > > 2.17.2 (Apple Git-113) > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-11 12:29 ` Eric Ren @ 2019-07-11 12:32 ` Dr. David Alan Gilbert 2019-07-11 12:49 ` Eric Ren 0 siblings, 1 reply; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-07-11 12:32 UTC (permalink / raw) To: Eric Ren, stefanha; +Cc: virtio-fs * Eric Ren (renzhen@linux.alibaba.com) wrote: > On Thu, Jul 11, 2019 at 12:25:58PM +0100, Dr. David Alan Gilbert wrote: > > * Eric Ren (renzhen@linux.alibaba.com) wrote: > > > SCMP_FLTATTR_CTL_TSYNC flag is only available on > > > Linux Kernel 3.17 or greater. So, conditional compile > > > to make virtio-fs work on older host kernel. > > > > > > Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> > > > --- > > > contrib/virtiofsd/seccomp.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c > > > index 4e388adc9c..5a28a90859 100644 > > > --- a/contrib/virtiofsd/seccomp.c > > > +++ b/contrib/virtiofsd/seccomp.c > > > @@ -11,6 +11,7 @@ > > > #include <errno.h> > > > #include <seccomp.h> > > > #include <glib.h> > > > +#include <linux/version.h> > > > #include "seccomp.h" > > > > > > static const int syscall_whitelist[] = { > > > @@ -92,9 +93,12 @@ void setup_seccomp(void) > > > err(1, "seccomp_init()"); > > > } > > > > > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) > > > > I don't really like using kernel versions, because sometimes downstreams > > backport stuff (I checked and it looks like RHEL7 did this somewhere > > around 7.5). > > > > If I understand correctly the right thing to do is check the > > SCMP_VER_MAJOR/MINOR/MICRO version defines; and I think tsync came in > > with 2.3.1. > > > > > + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater > > > if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { > > > err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); > > > > Also, what happens if this fails? e.g. I run it on an older kernel than > > it's built for; do we actually fail here or just print the error. > > So the result also applies if checking SCMP_VER_MAJOR/MINOR/MICRO > defines :-/ > > > > > Eithe rway, is it actually safe without this define - > Actually I don't know the exact effect of SCMP_FLTATR_CTL_TSYNC attr. > What if we gives a warning instead of error if failing to set it? > > > or does the thread > > which actually runs the work not get the support? > > Sorry, I fail to get your point here? I don't know seccomp that well (lets ask Stefan!), but my understanding of TSYNC is that it causes all threads to get the new seccomp rules not just the thread we're running in. So I'm worried that if we don't have TSYNC, some threads will run without the protection they need. Dave > Regards, > Eric > > > > > Dave > > > > > } > > > +#endif > > > > > > for (i = 0; i < G_N_ELEMENTS(syscall_whitelist); i++) { > > > if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW, > > > -- > > > 2.17.2 (Apple Git-113) > > > > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-11 12:32 ` Dr. David Alan Gilbert @ 2019-07-11 12:49 ` Eric Ren 0 siblings, 0 replies; 7+ messages in thread From: Eric Ren @ 2019-07-11 12:49 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: virtio-fs Hi, On Thu, Jul 11, 2019 at 01:32:54PM +0100, Dr. David Alan Gilbert wrote: > * Eric Ren (renzhen@linux.alibaba.com) wrote: [...] > > > Eithe rway, is it actually safe without this define - > > Actually I don't know the exact effect of SCMP_FLTATR_CTL_TSYNC attr. > > What if we gives a warning instead of error if failing to set it? > > > > > or does the thread > > > which actually runs the work not get the support? > > > > Sorry, I fail to get your point here? > > I don't know seccomp that well (lets ask Stefan!), but my understanding > of TSYNC is that it causes all threads to get the new seccomp rules > not just the thread we're running in. So I'm worried that if we don't > have TSYNC, some threads will run without the protection they need. Thanks, I get it. On older kernel host, it has not this support so I guess a warning message is the best we can do ;-) Regards, Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-10 13:06 [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support Eric Ren 2019-07-11 11:25 ` Dr. David Alan Gilbert @ 2019-07-12 8:04 ` Stefan Hajnoczi 2019-07-12 12:27 ` Eric Ren 1 sibling, 1 reply; 7+ messages in thread From: Stefan Hajnoczi @ 2019-07-12 8:04 UTC (permalink / raw) To: Eric Ren; +Cc: virtio-fs [-- Attachment #1: Type: text/plain, Size: 1337 bytes --] On Wed, Jul 10, 2019 at 09:06:42PM +0800, Eric Ren wrote: > SCMP_FLTATTR_CTL_TSYNC flag is only available on > Linux Kernel 3.17 or greater. So, conditional compile > to make virtio-fs work on older host kernel. > > Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> > --- > contrib/virtiofsd/seccomp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c > index 4e388adc9c..5a28a90859 100644 > --- a/contrib/virtiofsd/seccomp.c > +++ b/contrib/virtiofsd/seccomp.c > @@ -11,6 +11,7 @@ > #include <errno.h> > #include <seccomp.h> > #include <glib.h> > +#include <linux/version.h> > #include "seccomp.h" > > static const int syscall_whitelist[] = { > @@ -92,9 +93,12 @@ void setup_seccomp(void) > err(1, "seccomp_init()"); > } > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) > + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater > if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { > err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); > } > +#endif There are no threads when setup_seccomp() is invoked so the TSYNC attribute is not required. Please remove the TSYNC call. Then we don't need to worry about version or feature checks. Thanks, Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support 2019-07-12 8:04 ` Stefan Hajnoczi @ 2019-07-12 12:27 ` Eric Ren 0 siblings, 0 replies; 7+ messages in thread From: Eric Ren @ 2019-07-12 12:27 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: virtio-fs On Fri, Jul 12, 2019 at 10:04:46AM +0200, Stefan Hajnoczi wrote: > On Wed, Jul 10, 2019 at 09:06:42PM +0800, Eric Ren wrote: > > SCMP_FLTATTR_CTL_TSYNC flag is only available on > > Linux Kernel 3.17 or greater. So, conditional compile > > to make virtio-fs work on older host kernel. > > > > Signed-off-by: Eric Ren <renzhen@linux.alibaba.com> > > --- > > contrib/virtiofsd/seccomp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c > > index 4e388adc9c..5a28a90859 100644 > > --- a/contrib/virtiofsd/seccomp.c > > +++ b/contrib/virtiofsd/seccomp.c > > @@ -11,6 +11,7 @@ > > #include <errno.h> > > #include <seccomp.h> > > #include <glib.h> > > +#include <linux/version.h> > > #include "seccomp.h" > > > > static const int syscall_whitelist[] = { > > @@ -92,9 +93,12 @@ void setup_seccomp(void) > > err(1, "seccomp_init()"); > > } > > > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) > > + // SCMP_FLTATR_CTL_TSYNC flag is only available on Linux Kernel 3.17 or greater > > if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1) != 0) { > > err(1, "seccomp_attr_set(ctx, SCMP_FLTATTR_CTL_TSYNC, 1)"); > > } > > +#endif > > There are no threads when setup_seccomp() is invoked so the TSYNC > attribute is not required. Aha, great. > Please remove the TSYNC call. Then we don't need to worry about version > or feature checks. OK. Regards, Eric > > Thanks, > Stefan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-12 12:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-10 13:06 [Virtio-fs] [PATCH] virtiofsd: conditional compile seccomp flag support Eric Ren 2019-07-11 11:25 ` Dr. David Alan Gilbert 2019-07-11 12:29 ` Eric Ren 2019-07-11 12:32 ` Dr. David Alan Gilbert 2019-07-11 12:49 ` Eric Ren 2019-07-12 8:04 ` Stefan Hajnoczi 2019-07-12 12:27 ` Eric Ren
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.