[Ltrace-devel] commit 08677d0557 breaks libdl support

[Petr Machata]

Joe Damato <ice799 at gmail.com> writes:

> Hi -
>
> Petr: Your commit 08677d0557 appears to break ibdl support.
>
> The original code was extremely ugly and poorly written by me, but
> IIRC (I haven't looked at this code in a while) the lte array's first
> entry is the binary itself, while the library array is just libraries,
> so they are offset by one, hence the original unreadable code.
>
> OK to revert 08677d0557?

For reference, that's the commit in question:

@@ -241,12 +241,12 @@ hook_libdl_cb(void *data) {
 	lib_name = hook_data->lib_name;
 	addr = hook_data->addr;
 	lte = hook_data->lte;
 
 	if (library_num < MAX_LIBRARIES) {
-		library[library_num++] = strdup(lib_name);
 		lte[library_num].base_addr = addr;
+		library[library_num++] = strdup(lib_name);
 	}

Hmm, right, I see how I could think this was the proper fix, but your
explanation makes sense.  So yes to revert.

But the overrun was there and it may well have been casued by the
following.  In sysdeps/linux-gnu/events.c we see:

	} else if (!event.proc->libdl_hooked) {
		/* debug struct may not have been written yet.. */
		if (linkmap_init(event.proc, &main_lte) == 0) {

The variable main_lte is defined in ltrace-elf.c as plain "struct ltelf".

Then linkmap_init at sysdeps/linux-gnu/proc.c calls crawl_linkmap with
hook_libdl_cb for callback argument.  It passes the &main_lte as (struct
cb_data)->lte argument to the callback.  Then what the original,
unpatched code an quoted above does is:

		library[library_num++] = strdup(lib_name);
 		lte[library_num].base_addr = addr;

thus accessing element #1 (or maybe more, but not #0 in any case) of
what is just a structure.  Unless I'm missing more details, this would
explain the buffer overrun that I was seeing.

Thanks,
PM