Page MenuHomeFreeBSD

dtrace: conditionally load the systrace_linux klds when loading dtrace.
ClosedPublic

Authored by gallatin on Jan 9 2023, 3:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 5 2024, 7:50 PM
Unknown Object (File)
Oct 2 2024, 9:46 PM
Unknown Object (File)
Oct 2 2024, 9:14 PM
Unknown Object (File)
Oct 1 2024, 9:35 AM
Unknown Object (File)
Sep 27 2024, 9:17 AM
Unknown Object (File)
Aug 19 2024, 12:09 PM
Unknown Object (File)
Aug 16 2024, 7:49 AM
Unknown Object (File)
Aug 9 2024, 8:50 AM

Details

Summary

When dtrace starts, it tries to detect if the dtrace klds are loaded, and if not, it loads them by loading the dtraceall kld. This module depends on most dtrace modules, including systrace for the native freebsd and freebsd32 ABIs. However, it does not depend on the systrace_linux klds, as they in turn depend on the linux ABI klds, and we don't want to load an ABI module that the user has not explicitly requested. This can leave a naive user in a state where they think all syscall providers have been loaded, yet linux ABI syscalls are "invisible" to dtrace.

To fix this, check to see if the linux ABI modules are loaded. If they are, then load their systrace klds.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

Is this correct for 32-bit cases? (i.e., do we use linuxelf and systrace_linux there?)

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

I'm not sure, and I have no way to test this, as I have not run a 32-bit FreeBSD install in over a decade.

This should not make 32-bit platforms any worse off, as errors loading the modules are ignored. At worst, 32-bit platforms will maintain the status quo where systrace_linux is not loaded.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

Fair point, it's not worse than what's there now.

I was thinking perhaps of something like if (modfind("linux64elf") >= 0 || modfind("linuxelf") >= 0)) try loading both modules.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

In a case where somebody intended to load just 64b linux ABI, your suggestion would result in dtrace loading the 32-bit linux systrace which would then load the 32-bit linux ABI as a dependency.

It looks like linuxelf is the name of the 32-bit linux ABI on at least x86 (checked on ref14-i386)

I think the right thing the is:

		if (modfind("linuxelf") >= 0) {
			if (sizeof(void *) == 8)
				kldload("systrace_linux32");
			else
				kldload("systrace_linux");
		}

I'll test that (on amd64) and build-test with make universe..

Updated patch to load the proper systrace module when running on a 32-bit system.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

That works on 64b, and builds everywhere. And I *think* it should work on 32b.

emaste added inline comments.
cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1121

There's also a __LP64__ macro you could test (but the unused case here should be optimized out anyway, of course).

This revision is now accepted and ready to land.Jan 10 2023, 2:02 PM
cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

I think it'd be a bit better to check sizeof(void *) == 4) for the benefit of CHERI, where sizeof(void *) == 16.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1118–1119

Or perhaps test sizeof(long) == 8 like you do in D37707 (albeit at compile time).

I agree with @markj on using #ifdef's instead of sizeof(), so something like:

#if __SIZEOF_LONG__ == 8
    if (modfind("linux64elf") >= 0)
        kldload("systrace_linux");
    if (modfind("linuxelf") >= 0)
        kldload("systrace_linux32");
#else
    if (modfind("linuxelf") >= 0)
        kldload("sys trace_linux");
#endif

BTW, I really wish we would make "linuxelf" be the "native" Linux ABI and had linux32.ko instead of linux.ko on amd64 being i386. (*sigh*) That ship has probably sailed, but it would make this simpler had we done it correctly from the start.

I really wish we would make "linuxelf" be the "native" Linux ABI and had linux32.ko instead of linux.ko on amd64 being i386. (*sigh*) That ship has probably sailed

I don't think it's too late as long as someone wants to hand-hold the transition.

I really wish we would make "linuxelf" be the "native" Linux ABI and had linux32.ko instead of linux.ko on amd64 being i386. (*sigh*) That ship has probably sailed

I don't think it's too late as long as someone wants to hand-hold the transition.

You have to also fix at least truss if you rename things I think. Plus renaming linux64.ko to linux.ko is probably POLA by now. It may be doable, but it's fairly tedious probably, and certainly orthogonal to this change.

Update diff to use ifdefs and SIZEOF_LONG as suggested by reviewers.

This revision now requires review to proceed.Jan 21 2023, 7:34 PM
This revision is now accepted and ready to land.Jan 23 2023, 2:01 PM
trond.endrestol_ximalas.info added inline comments.
cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c
1124

Extra { causes compilation error.