* [PATCH] virtiofsd: Seccomp: Add 'send' for syslog @ 2020-11-02 15:07 Dr. David Alan Gilbert (git) 2020-11-02 15:11 ` Philippe Mathieu-Daudé 2020-11-02 15:17 ` Daniel P. Berrangé 0 siblings, 2 replies; 7+ messages in thread From: Dr. David Alan Gilbert (git) @ 2020-11-02 15:07 UTC (permalink / raw) To: qemu-devel, virtio-fs, amulmek1, stefanha From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> On ppc it looks like syslog ends up using 'send' rather than 'sendto'. Reference: https://github.com/kata-containers/kata-containers/issues/1050 Reported-by: amulmek1@in.ibm.com Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- tools/virtiofsd/passthrough_seccomp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c index eb9af8265f..672fb72a31 100644 --- a/tools/virtiofsd/passthrough_seccomp.c +++ b/tools/virtiofsd/passthrough_seccomp.c @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { /* Syscalls used when --syslog is enabled */ static const int syscall_whitelist_syslog[] = { + SCMP_SYS(send), SCMP_SYS(sendto), }; -- 2.28.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:07 [PATCH] virtiofsd: Seccomp: Add 'send' for syslog Dr. David Alan Gilbert (git) @ 2020-11-02 15:11 ` Philippe Mathieu-Daudé 2020-11-02 15:13 ` Dr. David Alan Gilbert 2020-11-02 15:18 ` Daniel P. Berrangé 2020-11-02 15:17 ` Daniel P. Berrangé 1 sibling, 2 replies; 7+ messages in thread From: Philippe Mathieu-Daudé @ 2020-11-02 15:11 UTC (permalink / raw) To: Dr. David Alan Gilbert (git), qemu-devel, virtio-fs, amulmek1, stefanha On 11/2/20 4:07 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > Reported-by: amulmek1@in.ibm.com > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_seccomp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > index eb9af8265f..672fb72a31 100644 > --- a/tools/virtiofsd/passthrough_seccomp.c > +++ b/tools/virtiofsd/passthrough_seccomp.c > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > /* Syscalls used when --syslog is enabled */ > static const int syscall_whitelist_syslog[] = { Is it worth restricting the syscall to POWER? #if defined(__powerpc64__) > + SCMP_SYS(send), #endif > SCMP_SYS(sendto), > }; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:11 ` Philippe Mathieu-Daudé @ 2020-11-02 15:13 ` Dr. David Alan Gilbert 2020-11-02 15:18 ` Daniel P. Berrangé 1 sibling, 0 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2020-11-02 15:13 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: virtio-fs, stefanha, qemu-devel, amulmek1 * Philippe Mathieu-Daudé (philmd@redhat.com) wrote: > On 11/2/20 4:07 PM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > > > Reported-by: amulmek1@in.ibm.com > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > index eb9af8265f..672fb72a31 100644 > > --- a/tools/virtiofsd/passthrough_seccomp.c > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > > > /* Syscalls used when --syslog is enabled */ > > static const int syscall_whitelist_syslog[] = { > > Is it worth restricting the syscall to POWER? > > #if defined(__powerpc64__) I don't think so, since it's legal for a libc to use either; so any other libc or architecture could choose either to use. Dave > > + SCMP_SYS(send), > > #endif > > > SCMP_SYS(sendto), > > }; > > > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:11 ` Philippe Mathieu-Daudé 2020-11-02 15:13 ` Dr. David Alan Gilbert @ 2020-11-02 15:18 ` Daniel P. Berrangé 2020-11-02 16:43 ` Stefan Hajnoczi 1 sibling, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2020-11-02 15:18 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: virtio-fs, stefanha, Dr. David Alan Gilbert (git), amulmek1, qemu-devel On Mon, Nov 02, 2020 at 04:11:44PM +0100, Philippe Mathieu-Daudé wrote: > On 11/2/20 4:07 PM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > > > Reported-by: amulmek1@in.ibm.com > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > index eb9af8265f..672fb72a31 100644 > > --- a/tools/virtiofsd/passthrough_seccomp.c > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > > > /* Syscalls used when --syslog is enabled */ > > static const int syscall_whitelist_syslog[] = { > > Is it worth restricting the syscall to POWER? That would be wrong, as this is needed on multiple arches. There is no real security benefit to restricting it either, as both syscalls give you broadly equivalent abilities. > > #if defined(__powerpc64__) > > > + SCMP_SYS(send), > > #endif > > > SCMP_SYS(sendto), > > }; 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 :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:18 ` Daniel P. Berrangé @ 2020-11-02 16:43 ` Stefan Hajnoczi 0 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2020-11-02 16:43 UTC (permalink / raw) To: Daniel P. Berrangé Cc: virtio-fs, Philippe Mathieu-Daudé, Dr. David Alan Gilbert (git), amulmek1, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1871 bytes --] On Mon, Nov 02, 2020 at 03:18:24PM +0000, Daniel P. Berrangé wrote: > On Mon, Nov 02, 2020 at 04:11:44PM +0100, Philippe Mathieu-Daudé wrote: > > On 11/2/20 4:07 PM, Dr. David Alan Gilbert (git) wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > > > > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > > > > > Reported-by: amulmek1@in.ibm.com > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > --- > > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > > index eb9af8265f..672fb72a31 100644 > > > --- a/tools/virtiofsd/passthrough_seccomp.c > > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > > > > > /* Syscalls used when --syslog is enabled */ > > > static const int syscall_whitelist_syslog[] = { > > > > Is it worth restricting the syscall to POWER? > > That would be wrong, as this is needed on multiple arches. > > There is no real security benefit to restricting it either, as both > syscalls give you broadly equivalent abilities. Restricting send to architectures where glibc slightly decreases the host kernel attack surface. I think it makes sense from a security perspective. There could be a send(2)-only bug in Linux isn't present in sendto(2). But there are other issues with seccomp - are we really sure send(2) is never called? Since we don't control the entire binary (share libraries are used, virtiofsd could be linked against another libc, etc) we may not have enough information to conclude that can be eliminated send(2). Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:07 [PATCH] virtiofsd: Seccomp: Add 'send' for syslog Dr. David Alan Gilbert (git) 2020-11-02 15:11 ` Philippe Mathieu-Daudé @ 2020-11-02 15:17 ` Daniel P. Berrangé 2020-11-02 18:30 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2020-11-02 15:17 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: virtio-fs, stefanha, qemu-devel, amulmek1 On Mon, Nov 02, 2020 at 03:07:50PM +0000, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > Reported-by: amulmek1@in.ibm.com > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tools/virtiofsd/passthrough_seccomp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > index eb9af8265f..672fb72a31 100644 > --- a/tools/virtiofsd/passthrough_seccomp.c > +++ b/tools/virtiofsd/passthrough_seccomp.c > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > /* Syscalls used when --syslog is enabled */ > static const int syscall_whitelist_syslog[] = { > + SCMP_SYS(send), > SCMP_SYS(sendto), > }; With glibc, syslog() calls __send() which for Linux target is implemented as: ssize_t __libc_send (int fd, const void *buf, size_t len, int flags) { #ifdef __ASSUME_SEND_SYSCALL return SYSCALL_CANCEL (send, fd, buf, len, flags); #elif defined __ASSUME_SENDTO_SYSCALL return SYSCALL_CANCEL (sendto, fd, buf, len, flags, NULL, 0); #else return SOCKETCALL_CANCEL (send, fd, buf, len, flags); #endif } We can see those defines being referenced vary per architecture: $ git grep -E '__ASSUME_SEND(TO)?_SYSCALL' sysdeps/ sysdeps/unix/sysv/linux/alpha/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/arm/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/hppa/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/i386/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL sysdeps/unix/sysv/linux/ia64/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/kernel-features.h:#define __ASSUME_SENDTO_SYSCALL 1 sysdeps/unix/sysv/linux/m68k/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL sysdeps/unix/sysv/linux/microblaze/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/mips/kernel-features.h:# define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/powerpc/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/s390/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL sysdeps/unix/sysv/linux/send.c:#ifdef __ASSUME_SEND_SYSCALL sysdeps/unix/sysv/linux/send.c:#elif defined __ASSUME_SENDTO_SYSCALL sysdeps/unix/sysv/linux/sendto.c:#ifdef __ASSUME_SENDTO_SYSCALL sysdeps/unix/sysv/linux/sh/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 sysdeps/unix/sysv/linux/sparc/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL So the patch is correct, but the commit message could be updated becase this isn't specific to PPC. Any platform except x86, s490, m68k will use send() rather than sendto() based on what I see here. With any commit message update, you can add Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtiofsd: Seccomp: Add 'send' for syslog 2020-11-02 15:17 ` Daniel P. Berrangé @ 2020-11-02 18:30 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2020-11-02 18:30 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: virtio-fs, qemu-devel, stefanha, amulmek1 * Daniel P. Berrangé (berrange@redhat.com) wrote: > On Mon, Nov 02, 2020 at 03:07:50PM +0000, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > On ppc it looks like syslog ends up using 'send' rather than 'sendto'. > > > > Reference: https://github.com/kata-containers/kata-containers/issues/1050 > > > > Reported-by: amulmek1@in.ibm.com > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > index eb9af8265f..672fb72a31 100644 > > --- a/tools/virtiofsd/passthrough_seccomp.c > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > @@ -118,6 +118,7 @@ static const int syscall_whitelist[] = { > > > > /* Syscalls used when --syslog is enabled */ > > static const int syscall_whitelist_syslog[] = { > > + SCMP_SYS(send), > > SCMP_SYS(sendto), > > }; > > With glibc, syslog() calls __send() which for Linux target is implemented > as: > > > ssize_t > __libc_send (int fd, const void *buf, size_t len, int flags) > { > #ifdef __ASSUME_SEND_SYSCALL > return SYSCALL_CANCEL (send, fd, buf, len, flags); > #elif defined __ASSUME_SENDTO_SYSCALL > return SYSCALL_CANCEL (sendto, fd, buf, len, flags, NULL, 0); > #else > return SOCKETCALL_CANCEL (send, fd, buf, len, flags); > #endif > } > > We can see those defines being referenced vary per architecture: > > $ git grep -E '__ASSUME_SEND(TO)?_SYSCALL' sysdeps/ > sysdeps/unix/sysv/linux/alpha/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/arm/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/hppa/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/i386/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL > sysdeps/unix/sysv/linux/ia64/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/kernel-features.h:#define __ASSUME_SENDTO_SYSCALL 1 > sysdeps/unix/sysv/linux/m68k/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL > sysdeps/unix/sysv/linux/microblaze/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/mips/kernel-features.h:# define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/powerpc/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/s390/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL > sysdeps/unix/sysv/linux/send.c:#ifdef __ASSUME_SEND_SYSCALL > sysdeps/unix/sysv/linux/send.c:#elif defined __ASSUME_SENDTO_SYSCALL > sysdeps/unix/sysv/linux/sendto.c:#ifdef __ASSUME_SENDTO_SYSCALL > sysdeps/unix/sysv/linux/sh/kernel-features.h:#define __ASSUME_SEND_SYSCALL 1 > sysdeps/unix/sysv/linux/sparc/kernel-features.h:# undef __ASSUME_SENDTO_SYSCALL > > > So the patch is correct, but the commit message could be updated becase > this isn't specific to PPC. Any platform except x86, s490, m68k will > use send() rather than sendto() based on what I see here. > > With any commit message update, you can add > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Thanks, changed to: On ppc, and some other archs, it looks like syslog ends up using 'send' rather than 'sendto'. Dave > > > 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 :| > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-02 18:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-02 15:07 [PATCH] virtiofsd: Seccomp: Add 'send' for syslog Dr. David Alan Gilbert (git) 2020-11-02 15:11 ` Philippe Mathieu-Daudé 2020-11-02 15:13 ` Dr. David Alan Gilbert 2020-11-02 15:18 ` Daniel P. Berrangé 2020-11-02 16:43 ` Stefan Hajnoczi 2020-11-02 15:17 ` Daniel P. Berrangé 2020-11-02 18:30 ` Dr. David Alan Gilbert
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).