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

[Kris Van Hees]

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>

> ---
>  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
> +
> +void *seccomp_fill_memory_really_malloc(size_t size);
> +void seccomp_fill_memory_really_free(void *ptr);
> +
> +#endif
> +
> -- 
> 2.39.1.268.g9de2f9a303
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel