[DTrace-devel] [PATCH 1/5] uprobes, libproc: handle uprobes to procs in different fs namespaces

[Nick Alcock]

On 13 May 2023, Kris Van Hees told this:

> On Mon, Apr 24, 2023 at 08:08:12PM +0100, Nick Alcock via DTrace-devel wrote:
>> The specification for a uprobe includes a *mapping name*, which is
>> documented as the "path to [the] executable or library" where the probe
>> is placed.  Unstated in the documentation is what filesystem namespace
>> this path is relative to, because -- obviously enough -- it's relative
>> to the task placing the uprobe.
>> 
>> Unfortunately we get our mapping names, ultimately, from the names in
>> /proc/$pid/maps, and those names are relative to the fs namespace of the
>> process being queried.  So we cannot just use that name unmodified: the
>> file might not be there, or indeed anywhere accessible to the
>> prober. Worse yet, if we're really unlucky some *other* process might be
>> there and will have a probe stuffed in it at a thoroughly unsuitable
>> location. One can imagine an attacker setting up a mount namespace with
>> a /bin/su of their own in it, and putting a DTrace probe in it such that
>> we stick uprobes in the *real* /bin/su and oh dear. This can probably
>> only cause a crash, but it still feels very wrong.
>> 
>> Worse yet, when started by systemd dtprobed runs in a non-default
>> filesystem namespace in which /home is inaccessible: put all this
>> together and you can't create DTrace probes in binaries you compile
>> yourself under /home, unless you restart dtprobed yourself outside
>> systemd's aegis (or hack the service file to turn off ProtectHome).
>> 
>> So fix this by using the map names from /proc/$pid/map_files, which we
>> already acquire for the purposes of opening the ELF executable in
>> invasive mode.  For that, we can fall back to the old GETMAPFD approach,
>> but given that that is an out-of-tree patch and was dead, and we're not
>> even looking for an fd here but rather a filename, and because the
>> conseqeuences of just using a filename out of /proc/$pid/maps are so
>> bad, we'll simply not implement a fallback here.  USDT requires
>> map_files built into the kernel and that's all.
>
> So the end of this paragraph begs the question: in what circumstances (if
> any) is map_files *not* built into the kernel?  Is it is matter of how old
> the kernel is?  How old are we talking about?  (Since I do not see any
> config option that enables/disables it.)

It's... complicated. Once upon a time, it was controlled by
CONFIG_CHECKPOINT_RESTORE, but Calvin Owens exposed it unconditionally
(requiring CAP_CHECKPOINT_RESTORE to access, which of course we have) in
4.3rc1.

So we can probably assume that /proc/$pid_map_files is always there for
any kernel DTrace cares about: it's dependent on procfs being built in
but of course we can't work at all without that.

We should probably just rip out the MAPFD fallback from libproc, given
that it's been dead for longer than BPF DTrace has existed.

>> This makes uprobe_events less informative (no filenames any more), but
>> so be it. We can always add a filename as another parameter or something
>> if we want to.
>> 
>> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
>
> Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com
>
> ... with comments/corrections as listed

Thanks!

>> ---
>>  libcommon/uprobes.c | 21 ++++++++++++++++++---
>>  libproc/Psymtab.c   | 27 ++++++++++++++++++++++++++-
>>  libproc/libproc.h   |  3 ++-
>>  3 files changed, 46 insertions(+), 5 deletions(-)
>> 
>> diff --git a/libcommon/uprobes.c b/libcommon/uprobes.c
>> index 8c3cc6280e9a1..396c12e493d10 100644
>> --- a/libcommon/uprobes.c
>> +++ b/libcommon/uprobes.c
>> @@ -27,6 +27,7 @@ uprobe_spec_by_addr(pid_t pid, ps_prochandle *P, uint64_t addr,
>>  	int			perr = 0;
>>  	char			*spec = NULL;
>>  	const prmap_t		*mapp, *first_mapp;
>> +	char			*mapfile_name = NULL;
>>  
>>  	if (!P) {
>>  		P = Pgrab(pid, 2, 0, NULL, &perr);
>> @@ -41,17 +42,31 @@ uprobe_spec_by_addr(pid_t pid, ps_prochandle *P, uint64_t addr,
>>  
>>  	first_mapp = mapp->pr_file->first_segment;
>>  
>> +	/*
>> +	 * Use a name in /proc/$pid/map_file: this will work even if the
>
> map_file -> map_files

Details, details! fixed.

-- 
NULL && (void)