[DTrace-devel] [PATCH 2/6] dtprobed: stop malloc making syscalls in the DOF parser child

Thu May 25 19:12:50 UTC 2023

On Thu, May 25, 2023 at 03:04:04PM -0400, Kris Van Hees via DTrace-devel wrote:
> On Mon, May 22, 2023 at 09:20:13PM +0100, Nick Alcock via DTrace-devel wrote:
> > If dtprobed receives DOF larger than the glibc mmap threshold (64KiB by
> > default), it will make an mmap() to allocate space for it in the parser
> > child.  Since it's in a seccomp jail, this is denied and the child is
> > killed.  Whoops.
> > 
> > Rejigging the entire parser child to avoid allocations is possible, but
> > needless: we can just ask glibc to never call mmap and keep enough space
> > free at the top of the heap for plausible allocations, make a big
> > allocation right before nailing ourselves into the jail, and keep
> > going. We have a (fairly small) maximum size for a given piece of DOF,
> > and there is no risk of heap fragmentation because we free everything
> > on every message received, so a fixed size (with respect to that
> > maximum) should always be enough.
> > 
> > (There is a slight extra cocmplexity here: we have to hide the malloc()
> > and free() for the big allocation in another translation unit (and turn
> > LTO off for that translation unit), or GCC will recognise that the
> > malloc/free are do-nothing, and optimize them away.)
> > 
> > (If glibc ever stops respecting M_MMAP_MAX or M_TRIM_THRESHOLD we'll be
> > in bigger trouble and might need our own simpleminded allocator, but
> > this doesn't seem at all likely in the near future at least.)
> > 
> > Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> 
> Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

Actually, small change needed (see below)

> > ---
> >  dtprobed/Build                |  3 ++-
> >  dtprobed/dtprobed.c           | 18 +++++++++++++++---
> >  dtprobed/seccomp-assistance.c | 18 ++++++++++++++++++
> >  dtprobed/seccomp-assistance.h | 15 +++++++++++++++
> >  4 files changed, 50 insertions(+), 4 deletions(-)
> >  create mode 100644 dtprobed/seccomp-assistance.c
> >  create mode 100644 dtprobed/seccomp-assistance.h
> > 
> > diff --git a/dtprobed/Build b/dtprobed/Build
> > index 7e3113fdd0b6f..fcaea2d324a1e 100644
> > --- a/dtprobed/Build
> > +++ b/dtprobed/Build
> > @@ -26,7 +26,7 @@ dtprobed_CPPFLAGS := -I. -Idtprobed -Ilibproc -Ilibcommon -Ilibport
> >  dtprobed_CFLAGS := $(shell pkg-config --cflags $(LIBFUSE))
> >  dtprobed_LIBS := -lcommon -lproc -lcommon -lport -lelf $(shell pkg-config --libs $(LIBFUSE))
> >  dtprobed_DEPS := libproc.a libcommon.a libport.a
> > -dtprobed_SOURCES := dtprobed.c
> > +dtprobed_SOURCES := dtprobed.c seccomp-assistance.c
> >  dtprobed_LIBSOURCES := libproc libcommon
> >  
> >  ifdef HAVE_LIBSYSTEMD
> > @@ -39,6 +39,7 @@ dtprobed_SOURCES += rpl_fuse_log.c
> >  endif
> >  
> >  dtprobed.c_CFLAGS := -Wno-pedantic
> > +seccomp-assistance.c_CFLAGS := -fno-lto
> >  endif
> >  
> >  install::
> > diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> > index 3725dba2fe1c6..9eadf8cffc15d 100644
> > --- a/dtprobed/dtprobed.c
> > +++ b/dtprobed/dtprobed.c
> > @@ -9,6 +9,7 @@
> >  #include <sys/wait.h>
> >  #include <errno.h>
> >  #include <fcntl.h>
> > +#include <malloc.h>
> >  #include <poll.h>
> >  #include <stdarg.h>
> >  #include <stdio.h>
> > @@ -52,6 +53,8 @@
> >  #include "dof_parser.h"
> >  #include "uprobes.h"
> >  
> > +#include "seccomp-assistance.h"
> > +
> >  #define DOF_MAXSZ 512 * 1024 * 1024
> >  
> >  static struct fuse_session *cuse_session;
> > @@ -250,12 +253,21 @@ dof_parser_start(int sync_fd)
> >  		 * closed all pipes that we might use to report it.  Just exit 1
> >  		 * and rely on the admin using strace :(
> >  		 *
> > -		 * Don't do any of this if debugging (but still run in a child
> > -		 * process).
> > +		 * Take additional measures to ensure that we can still do
> > +		 * sufficiently-large mallocs in the child without needing to
> > +		 * make any syscalls.
> > +		 *
> > +		 * Don't lock the process in jail if debugging (but still run in
> > +		 * a child process).
> >  		 */
> > -		if (!_dtrace_debug)
> > +		mallopt(M_MMAP_MAX, 0);
> > +		mallopt(M_TRIM_THRESHOLD, -1);
> > +		seccomp_fill_memory_really_free(seccomp_fill_memory_really_malloc(DOF_MAXSZ * 2));
> > +
> > +		if (!_dtrace_debug) {
> >  			if (syscall(SYS_seccomp, SECCOMP_SET_MODE_STRICT, 0, NULL) < 0)
> >  				_exit(1);
> > +		}
> >  
> >  		while (parse_dof(parser_in_pipe[0], parser_out_pipe[1]))
> >  			;
> > diff --git a/dtprobed/seccomp-assistance.c b/dtprobed/seccomp-assistance.c
> > new file mode 100644
> > index 0000000000000..02f36e1cc6aff
> > --- /dev/null
> > +++ b/dtprobed/seccomp-assistance.c
> > @@ -0,0 +1,18 @@
> > +/*
> > + * Oracle Linux DTrace; assist seccomp, fool the compiler
> > + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> > + * Licensed under the Universal Permissive License v 1.0 as shown at
> > + * http://oss.oracle.com/licenses/upl.
> > + */
> > +
> > +#include <stdlib.h>
> > +
> > +void *seccomp_fill_memory_really_malloc(size_t size)
> > +{
> > +	return malloc(size);
> > +}
> > +
> > +void seccomp_fill_memory_really_free(void *ptr)
> > +{
> > +	free(ptr);
> > +}
> > diff --git a/dtprobed/seccomp-assistance.h b/dtprobed/seccomp-assistance.h
> > new file mode 100644
> > index 0000000000000..8a82a0f60b82c
> > --- /dev/null
> > +++ b/dtprobed/seccomp-assistance.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * Oracle Linux DTrace; assist seccomp, fool the compiler
> > + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> > + * Licensed under the Universal Permissive License v 1.0 as shown at
> > + * http://oss.oracle.com/licenses/upl.
> > + */
> > +
> > +#ifndef	_SECCOMP_ASSISTANCE_H
> > +#define	_SECCOMP__ASSISTANCE_H

Shouldn't this be _SECCOMP_ASSISTANCE_H ?

> > +
> > +void *seccomp_fill_memory_really_malloc(size_t size);
> > +void seccomp_fill_memory_really_free(void *ptr);
> > +
> > +#endif
> > +
> > -- 
> > 2.39.1.268.g9de2f9a303