[PATCH 0/1] prefcnt: guard against NULL function parameters

[PATCH 0/1] prefcnt: guard against NULL function parameters

[Håvard F. Aasen]

When prefcnt is invoked without an argument, it attempts to process the
debugging information for the running kernel. Depending on the
available BTF/ELF data, the function-parameter list may contain a NULL
entry.

I'm not convinced that the proposed patch is the only, or best way to
fix this issue. Both cus__load_running_kernel() and the total expansion
of list_for_each_entry() macro might be candidates for a more elegant
solution.

Håvard F. Aasen (1):
  prefcnt: guard against NULL function parameters

 prefcnt.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.53.0

[PATCH 1/1] prefcnt: guard against NULL function parameters

[Håvard F. Aasen]

When prefcnt is started without an input file, it loads the running
kernel's debugging information through the existing vmlinux/BTF/ELF
loading path. While walking the functions in the resulting
compilation units, some function parameter entries can be represented
by a NULL pointer.

refcnt_function() passes each entry from function->proto.parms to
refcnt_parameter(). refcnt_parameter() dereferences the entry when it
checks parameter->tag.type, causing prefcnt to terminate with a
segmentation fault if the entry is NULL.

For example, the failure is visible in the following backtrace:

  #0  0x0000555555555670 in refcnt_function (function=0x5555563e7a40, cu=0x555555559070)
    at /srv/git/dwarves/prefcnt.c:113
        __mptr = 0x555555559070
        parameter = 0x0
  #1  0x00005555555556fe in cu_refcnt_iterator (cu=0x555555559070, cookie=0x0) at /srv/git/dwarves/prefcnt.c:125
        pos = 0x5555563e7a40
        id = 55272
  #2  0x00007ffff7f079a6 in cus__for_each_cu (cus=0x555555559010, iterator=0x5555555556aa <cu_refcnt_iterator>,
    cookie=0x0, filter=0x0) at /srv/git/dwarves/dwarves.c:2094
        cu = 0x555555559070
        pos = 0x555555559070
  #3  0x000055555555588d in main (argc=1, argv=0x7fffffffde78) at /srv/git/dwarves/prefcnt.c:162
        err = 0
        cus = 0x555555559010

Treat a NULL parameter entry as an absent parameter and return before
accessing its members. This allows us to continue processing the
remaining debug information.

Signed-off-by: Håvard F. Aasen <havard.f.aasen@pfft.no>
---
 prefcnt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/prefcnt.c b/prefcnt.c
index 37ca18f..b027f4d 100644
--- a/prefcnt.c
+++ b/prefcnt.c
@@ -37,6 +37,10 @@ static void refcnt_member(struct class_member *member, const struct cu *cu)
 static void refcnt_parameter(const struct parameter *parameter,
 			     const struct cu *cu)
 {
+	if (parameter == NULL) {
+		return;
+	}
+
 	if (parameter->tag.type != 0) { /* if not void */
 		struct tag *type = cu__type(cu, parameter->tag.type);
 		if (type != NULL)
-- 
2.53.0

Re: [PATCH 1/1] prefcnt: guard against NULL function parameters

[Alan Maguire]

> When prefcnt is started without an input file, it loads the running
> kernel's debugging information through the existing vmlinux/BTF/ELF
> loading path. While walking the functions in the resulting
> compilation units, some function parameter entries can be represented
> by a NULL pointer.
> 
> refcnt_function() passes each entry from function->proto.parms to
> refcnt_parameter(). refcnt_parameter() dereferences the entry when it
> checks parameter->tag.type, causing prefcnt to terminate with a
> segmentation fault if the entry is NULL.

I still see a segmentation fault when running prefcnt on /sys/kernel/btf/vmlinux
with this change; I think the right answer here is to fix the caller by
using function__for_each_parameter() which will correctly use the ftype
to get parameters for BTF, i.e.

diff --git a/prefcnt.c b/prefcnt.c
index 37ca18f..86fceba 100644
--- a/prefcnt.c
+++ b/prefcnt.c
@@ -106,7 +106,7 @@ static void refcnt_function(struct function *function, const struct cu *cu)
                        refcnt_tag(type, cu);
        }

-       list_for_each_entry(parameter, &function->proto.parms, tag.node)
+       function__for_each_parameter(function, cu, parameter)
                refcnt_parameter(parameter, cu);

        refcnt_lexblock(&function->lexblock, cu);


Can you send a v2? Thanks!