Re: [PATCH dwarves v4 2/2] pahole: Inject kfunc decl tags into BTF

[Eduard Zingerman <eddyz87@xxxxxxxxx>]

On Wed, 2024-02-28 at 09:07 -0700, Daniel Xu wrote:
> Hi Eduard,
>
> Apologies for long delay - life has been busy.

Hi Daniel,

No problem, thank you for reaching back.

[...]

> > > +static char *get_func_name(const char *sym)
> > > +{
> > > +	char *func, *end;
> > > +
> > > +	if (strncmp(sym, BTF_ID_FUNC_PFX, sizeof(BTF_ID_FUNC_PFX) - 1))
> > > +		return NULL;
> > > +
> > > +	/* Strip prefix */
> > > +	func = strdup(sym + sizeof(BTF_ID_FUNC_PFX) - 1);
> > > +
> > > +	/* Strip suffix */
> > > +	end = strrchr(func, '_');
> > > +	if (!end || *(end - 1) != '_') {
> > 
> > Nit: this would do out of bounds access on malformed input
> >      "__BTF_ID__func___"
> 
> I think this is actually ok. Reason is we have the strncmp() above
> so we know the prefix is there. Then the strdup() in the malformed cased
> returns empty string. And strrchr() will then return NULL, so we don't
> enter the branch.
> 
> I tested it with: https://pastes.dxuuu.xyz/c3j4kk
> 
>         $ gcc test.c
>         dxu@kashmir~/scratch $ ./a.out
>         name=(null)
> 

The test is for "__BTF_ID__func__", but nitpick is for "__BTF_ID__func___"
(three underscores in the end).

E.g. here is a repro:

--- 8< ---------------------------------------------------------------

$ cat test.c

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

#define BTF_ID_FUNC_PFX         "__BTF_ID__func__"

static char *get_func_name(const char *sym)
{
        char *func, *end;

        if (strncmp(sym, BTF_ID_FUNC_PFX, sizeof(BTF_ID_FUNC_PFX) - 1))
                return NULL;

        /* Strip prefix */
        func = strdup(sym + sizeof(BTF_ID_FUNC_PFX) - 1);

        /* Strip suffix */
        end = strrchr(func, '_');
        if (!end || *(end - 1) != '_') {
                free(func);
                return NULL;
        }
        *(end - 1) = '\0';

        return func;
}

int main(int argc, char *argv[]) {
	if (argc < 2)
		return -1;

	printf("name='%s;\n", get_func_name(argv[1]));
	return 0;
}

$ gcc -g test.c

$ valgrind ./a.out __BTF_ID__func___

==16856== Memcheck, a memory error detector
==16856== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==16856== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==16856== Command: ./a.out __BTF_ID__func___
==16856== 
==16856== Invalid read of size 1
==16856==    at 0x4011EB: get_func_name (test.c:19)
==16856==    by 0x401244: main (test.c:32)
==16856==  Address 0x4a7e03f is 1 bytes before a block of size 2 alloc'd
==16856==    at 0x4845784: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16856==    by 0x492176D: strdup (in /usr/lib64/libc.so.6)
==16856==    by 0x4011C2: get_func_name (test.c:15)
==16856==    by 0x401244: main (test.c:32)
==16856== 
name='(null);
==16856== 
==16856== HEAP SUMMARY:
==16856==     in use at exit: 0 bytes in 0 blocks
==16856==   total heap usage: 2 allocs, 2 frees, 1,026 bytes allocated
==16856== 
==16856== All heap blocks were freed -- no leaks are possible
==16856== 
==16856== For lists of detected and suppressed errors, rerun with: -s
==16856== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

--------------------------------------------------------------- >8 ---

Thanks,
Eduard