PLT stub names not being resolved properly

[haystack-ia]

Work environment

Questions	Answers
OS/arch/bits (mandatory)	Arch x86_64
File format of the file you reverse (mandatory)	ELF
Architecture/bits of the file (mandatory)	arm 32
r2 -v full output, not truncated (mandatory)	radare2 4.6.0-git 25551 @ linux-x86-64 git.4.4.0-580-g7066f58d8
commit: 7066f58 build: 2020-08-25__09:23:38

Expected behavior

When a function exported by a shared object file is called from within that shared object file, the function name for the stub in the plt section should be named properly.

Example: in libcurl, the function Curl_getaddrinfo_ex is called from the function Curl_ipv4_resolve_r. That function call should appear as sym.imp.Curl_getaddrinfo_ex.

Actual behavior

Instead of sym.imp.Curl_getaddrinfo_ex, the function is flagged fcn.000074fc. This makes analysis a bit painful.

The address that the plt stub points to is correctly identified as reloc.Curl_getaddrinfo_ex, so the relocation is being correctly ID'd (I've also double-checked with Ghidra).

Steps to reproduce the behavior

Asciinema: https://asciinema.org/a/o4UkBFPskH5rhJZ8lwjuNTKJC

Representative file here: libcurl.zip

Here is a screenshot from ghidra showing the same address (though, interestingly, it's mapped at 0x174fc instead of 0x74fc):

[XVilka]

See also #17300 cc @08a

[ghost]

@XVilka Yep again the same bug, i am writing a python script to see if an analysis cmd could fix this

[haystack-ia]

Yep again the same bug, i am writing a python script to see if an analysis cmd could fix this

@08a

You may find this script useful maybe: https://github.com/haystack-ia/radare-funhouse/blob/master/plt_fixup.py

This is how I bandaid-ed the problem for my own analysis. I don't have an architecture-agnostic solution though.

[ghost]

@haystack-ia Yep your analysis is a good entry Thanks

[ghost]

If you want to look at the implementation.
https://github.com/08A/radare2_misc/blob/master/plt_got.py
It doesn't work on some arm binary because (bug ?)

[trufae]

Wonder if this was fixed

[trufae]

yep its fixed

[mauriziopapini]

@trufae is it in 5.8.8? Cannot find it.

[trufae]

Yes. There are tests too

[grgalex]

What commit on master implements the fix?

I looked at the diffs of the commits that GH list above as referencing this issue, but I don't see their proposed changes in master.

For example, in libr/bin/p/bin_elf.inc.c, I can't find any reference to is_internal, as commit d98edd0 (which is not merged) suggests.

[grgalex]

cc: @trufae

[trufae]

its referenced in the issue already

[grgalex]

@trufae Where? I can't find it :(

[grgalex]

I'm quite sure that there is no commit on master that fixes this bug.

[trufae]

i dont see any problem here, or am i missing something?

[grgalex]

I have the following library:

int add2(int a, int b);

int add(int a, int b) {
    return add2(a,b);
}

int add2(int a, int b) {
    return a + b;
}

1. Compile with gcc:

gcc a.c -fPIC -shared -o a.so

2. Load with r2

r2 -A a.so

3. Generate the global call graph as png

> agCw graph.png

The call graph I get is the following:

The call graph I expected to get has an arrow from sym.add to sym.add2. Instead, there is an arrow from sym.add to fcn.00001030.

I think this is the exact same case as the one reported in this issue.

Also, I can't find the fixing commit/PR or tests for this issue. Can you point to them? I have a slight suspicion that something is wrong here.

[grgalex]

@trufae Did you have time to take a look at the above comment?

I think you forgot to merge the fix commits to master. I'm not joking.

There's no occurence of the string is_internal in any file on master.

I found the commit that adds the test (e219383).

But there's no commit that adds a fix. I think the test passes by chance.

[trufae]

git log shows that this test was marked as FIXED in 93dbc62 why the string "is_internal" should matter? can you share the library you are building instead of sharing the steps? because i'm compiling it on an ubuntu-x64 (because i assume this is your environment) and what i get is exactly your output, and if you look at the code generated by GCC there's no direct call from add to add2. but instead, it's calling a stub that calls add2. this stub depends on a reloc to be patched, so i'm afraid the callgraph is correct and the elf parsing is also correct.

But there are still other issues to be fixed in the elf parser as well as in the relocs patching like this one: #21117

but i dont think that's the same issue as the one you are refering

[trufae]

you are indeed correct that the commit you are referencing was not merged. i just pushed a pr with the rebased change to see what happens. but im unsure of the reason the change wasn't included.
ref: #23805

[trufae]

ive improved the autonaming implementation to make the callgraph look better here. #23806

[grgalex]

Does #23806 need to be applied on top of #23805?

[grgalex]

By the way, regarding the example with add and add2, I think it's the same as the one in the issue.

In the sense that an internal call to a public symbol add2 gets resolved to fcn.XXXX instead of sym.add2.

By the way, am I right to expect that the public callgraph should contain nodes for both sym.add and sym.add2 and an arrow from sym.add to sym.add2? Or am I misunderstanding something?

In my understanding, the global callgraphs should show all available symbols (== potential "entrypoints" from external code) in the library (either local or public) and any calls they make.

[trufae]

i think the callgraph needs some love. and part of it would be to add support for tailcall optimization rendered as nodes. right now iirc it's only following CALLs, but not those jumps.

i think you should open a separate issue to track this, but the fix should be relatively simple in case you want to contribute that can be a good easy issue to tackle. btw, ive added the add.so in the testbins repo . im using it for testing the autonaming and the callgraph issue you mentioned.

i can try to find some time to do it later this week or so, but rn im busy with other fronts

[grgalex]

I'm trying to verify that I've understood what the expected output of agCw a.so is, and then we can fix it :)

Should the global callgraph also contain nodes that don't make calls? (in this case add2?). In my opinion yes, but I might be misunderstanding something.

[trufae]

when a function just calls another function and returns the result of that other function, compilers do this "tailcall" optimization, which basically means, replacing a CALL with a JMP and remove the ret. r2 is able to understand those constructions, but from the assembly point of view this is just a JMP, which means that, depending on how you see the code, this can be a jump to another function or a call+ret.

the code that constructs the callgraph is just following CALL instructions for that, it may be also taking into account those tailcalls jumps. so yeah, i think that the graph should look like this at the end:

flowchart TB
  add --> fcn.1050
  fcn.1050 -->add2
  add2

Loading

[grgalex]

Ok, but even when there's no tailcall, (i.e., add calls add2 among other things and functions), I don't get the output I expected.
Source:

int add2(int a, int b);

int add(int a, int b) {
        int x, y, z;
        x = add2(a, b);
        y = add2(a, b);
        return x * y;
}

int add2(int a, int b) {
        int sum;
        sum = a + b;
        return sum;
}

Compile:

gcc a.c -fPIC -shared -o a.so

In this case, agCw outputs:

Which faces the exact same problems:

1. add2 is completely missing.
2. There's no call from add to add2.