dtprobed: force_reparse signal safety

[thesamesam]

In dtprobed/dtprobed.c, we do:

static void
force_reparse(int sig)
{
        fuse_log(FUSE_LOG_DEBUG, "Forced reparse\n");
        reparse_dof(parser_out_pipe, parser_in_pipe, process_dof, 1);
        fuse_log(FUSE_LOG_DEBUG, "Forced reparse complete\n");
}

int
main(...)
{
		/* ... */

        /*
         * When doing (in-tree) testing, use a shorter cleanup interval, and
         * hook up a signal allowing the test scripts to force a DOF cleanup.
         */
        if (getenv("_DTRACE_TESTING")) {
                struct sigaction tmp = {0};

                cleanup_interval = 5;
                tmp.sa_handler = force_reparse;
                tmp.sa_flags = SA_RESTART;
                (void) sigaction(SIGUSR2, &tmp, NULL);
                testing = 1;
        }
	/* ... */
}

force_reparse calls fuse_log which doesn't appear guaranteed to be AS-safe (https://github.com/libfuse/libfuse/blob/d30247c36dadd386b994cd47ad84351ae68cc94c/lib/fuse_log.c#L77, it might even try to syslog), but worse, we call reparse_dof which does bunch of I/O and more complex things.

This isn't likely to be a big problem in reality, not least because it's gated by an envvar, but still.

[nickalcock]

Yeah, as the envvar suggests it's a pure testing feature. Nothing outside the testsuite should ever use it, and the dtprobed that is hit by this signal is running one very specific test and except on a hilariously slow (XZ81-class) system is not going to be doing a reparse_dof when the signal hits. Anyone running with _DTRACE_TESTING set is going to find much odder things going on (dtrace's very output format changes).

Can we just say "don't do that then"? We could always add an extra check that dtprobed is running out of the source tree, but that means more file I/O as root...

[nickalcock]

I'm happy to introduce some other way to say "force reparsing now so we can test it", but the only one that springs to mind is another ioctl (!) which is much harder from a shell script.

[kvanhees]

On Wed, Aug 28, 2024 at 08:41:02AM -0700, Nick Alcock wrote: I'm happy to introduce some other way to say "force reparsing now so we can test it", but the only one that springs to mind is another ioctl (!) which is much harder from a shell script.

How about compiling dtprobed without this debugging code in general, and only compile one (perhaps a dtprobed-tst or something) with this code compiled in that isused for the test? That renders the test development-only but I think that might perhaps be preferable to having this code in a production version?

…

[thesamesam]

"Don't do that then" would be OK but I think I prefer Kris' suggestion to just never install the code to begin with. I don't feel strongly.