[PATCH 1/2] dtprobed: handle a crashed parser child better

[PATCH 1/2] dtprobed: handle a crashed parser child better

[Nick Alcock]

When a parser child crashes and is restarted, its file handles
might change: we should pick up the new fhes if so.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 dtprobed/dtprobed.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 86865eb467b67..c0597b7329d37 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -769,7 +769,13 @@ process_dof(pid_t pid, int out, int in, dev_t dev, ino_t inum, dev_t exec_dev,
 		if (!provider) {
 			if (tries++ > 1)
 				goto err;
+			/*
+			 * Tidying reopens the parser in and out pipes: catch
+			 * up with this.
+			 */
 			dof_parser_tidy(1);
+			out = parser_out_pipe;
+			in = parser_in_pipe;
 			continue;
 		}
 		if (provider->type != DIT_PROVIDER && provider->type != DIT_EOF)
-- 
2.47.1.279.g84c5f4e78e

[PATCH 2/2] libcommon: turn off FORTIFY_SOURCE more aggressively

[Nick Alcock]

This is incompatible with dof_parser_t's use of trailing strings of
variable length, and buys us nothing anyway since we're running in a
seccomped parser child and any buffer overruns are harmless and cannot
result in anything other than a dying child.  We were already -U'ing
FORTIFY_SOURCE in libcommon_CPPFLAGS, but that precedes CFLAGS on the
command line, so if the build system passes -D_FORTIFY_SOURCE there, it
still takes effect.

Use _NOCFLAGS and _NOCPPFLAGS to explicitly disable every level of
fortification that currently exists.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 libcommon/Build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libcommon/Build b/libcommon/Build
index fd54dc044f9af..00c0d46d125e4 100644
--- a/libcommon/Build
+++ b/libcommon/Build
@@ -10,4 +10,6 @@ libcommon_TARGET = libcommon
 libcommon_DIR := $(current-dir)
 libcommon_CPPFLAGS := -Ilibcommon -Ilibproc -U_FORTIFY_SOURCE
 libcommon_SOURCES = dof_parser.c dof_parser_host.c dt_list.c
+libcommon_NOCFLAGS := -D_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=3
+libcommon_NOCPPFLAGS := -D_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=3
 libcommon_LIBSOURCES = libcommon
-- 
2.47.1.279.g84c5f4e78e

Re: [PATCH 2/2] libcommon: turn off FORTIFY_SOURCE more aggressively

[Kris Van Hees]

On Thu, Jan 16, 2025 at 09:33:32PM +0000, Nick Alcock wrote:
> This is incompatible with dof_parser_t's use of trailing strings of
> variable length, and buys us nothing anyway since we're running in a
> seccomped parser child and any buffer overruns are harmless and cannot
> result in anything other than a dying child.  We were already -U'ing
> FORTIFY_SOURCE in libcommon_CPPFLAGS, but that precedes CFLAGS on the
> command line, so if the build system passes -D_FORTIFY_SOURCE there, it
> still takes effect.
> 
> Use _NOCFLAGS and _NOCPPFLAGS to explicitly disable every level of
> fortification that currently exists.
> 
> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libcommon/Build | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libcommon/Build b/libcommon/Build
> index fd54dc044f9af..00c0d46d125e4 100644
> --- a/libcommon/Build
> +++ b/libcommon/Build
> @@ -10,4 +10,6 @@ libcommon_TARGET = libcommon
>  libcommon_DIR := $(current-dir)
>  libcommon_CPPFLAGS := -Ilibcommon -Ilibproc -U_FORTIFY_SOURCE
>  libcommon_SOURCES = dof_parser.c dof_parser_host.c dt_list.c
> +libcommon_NOCFLAGS := -D_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=3
> +libcommon_NOCPPFLAGS := -D_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=3
>  libcommon_LIBSOURCES = libcommon
> -- 
> 2.47.1.279.g84c5f4e78e
> 

Re: [DTrace-devel] [PATCH 1/2] dtprobed: handle a crashed parser child better

[Kris Van Hees]

On Thu, Jan 16, 2025 at 09:33:31PM +0000, Nick Alcock via DTrace-devel wrote:
> When a parser child crashes and is restarted, its file handles
> might change: we should pick up the new fhes if so.
> 
> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  dtprobed/dtprobed.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index 86865eb467b67..c0597b7329d37 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -769,7 +769,13 @@ process_dof(pid_t pid, int out, int in, dev_t dev, ino_t inum, dev_t exec_dev,
>  		if (!provider) {
>  			if (tries++ > 1)
>  				goto err;
> +			/*
> +			 * Tidying reopens the parser in and out pipes: catch
> +			 * up with this.
> +			 */
>  			dof_parser_tidy(1);
> +			out = parser_out_pipe;
> +			in = parser_in_pipe;
>  			continue;
>  		}
>  		if (provider->type != DIT_PROVIDER && provider->type != DIT_EOF)
> -- 
> 2.47.1.279.g84c5f4e78e
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel