Fix dmht for glibc caused by wrong tcache offset and definition

[x0urc3]

Your checklist for this pull request

• I've read the guidelines for contributing to this repository
• I made sure to follow the project's coding style
• I've added tests that prove my fix is effective or that my feature works (if possible)
• I've updated the documentation and the radare2 book with the relevant information (if needed)

Detailed description

Related to PR #16239, dmht does not show freed tcache chunk due to wrong tcache size and definition.
...

Test plan

• Result has been compared with gdb-gef
• Pass added test

[x0urc3]

Hmmm. both dmht and test are working on my machine

[XVilka]

@x0urc3 different version of glibc?

[x0urc3]

@x0urc3 different version of glibc?

Might explain it. How could I find the glibc version in Travis?

[XVilka]

It's 2.23 in Travis (Ubuntu Xenial) it seems: https://repology.org/project/glibc/versions

[x0urc3]

One more thing is the expected output does not follow the sequence of command

r2 -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Q -d -c "dcu main;dmht~Tcache;20dso;dmht~Tcache" ../bins/elf/simple_malloc_x86_64 2>/dev/null
p=3
Tcache main arena @ 0x7f6d1e4b2b80
Tcache main arena @ 0x7f6d1e4b2b80

-p=3 should come after the first dmht~Tcache

[x0urc3]

It's 2.23 in Travis (Ubuntu Xenial) it seems: https://repology.org/project/glibc/versions

This caused issue because dbg.glibc.tcache should only be enabled for glibc >2.26

[radare]

can you fix this?

[x0urc3]

can you fix this?

Yes. I will create another PR for glibc version detection to set dbg.glibc.tcache

[radare]

resolve the conflicts

[radare]

still conflicting

[radare]

still conflicting

[radare]

[x0urc3]

Forgot to rebase

[x0urc3]

Failed one of the added test. I'm recreating a ubuntu xenial image to test locally

[radare]

[x0urc3]

• Could not reproduce using docker debian/xenial:latest image.
• Tested by running the command in r2 and r2r.js
• Any suggestion?

[radare]

Trusty?

…

On 20 Mar 2020, at 15:38, Khairulmizam Samsudin ***@***.***> wrote:  Could not reproduce using docker debian/xenial:latest image. Any suggestion? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[XVilka]

Xenial is the default Travis configuration, and we do not specify the distro in YAML, so should be it.

[XVilka]

Ideally, it should be tested with a sequence of om and wx commands, for different glibc versions. Relying on the runtime for testing is wrong.

[radare]

ping

[x0urc3]

I'm creating a test as suggested by @XVilka. May take some time

[radare]

keep us updated

[radare]

red

[x0urc3]

Ideally, it should be tested with a sequence of om and wx commands, for different glibc versions. Relying on the runtime for testing is wrong.

glibc struct for heap management requires around 3kB. Should I just add the memory dump to radare2-testbins rather than using wx?

[XVilka]

Yes, you can add it testbins of course

…

On Wed, Mar 25, 2020, 04:25 Khairulmizam Samsudin ***@***.***> wrote: Ideally, it should be tested with a sequence of om and wx commands, for different glibc versions. Relying on the runtime for testing is wrong. glibc struct for heap requires around 3kB. Should I just add the memory dump to radare2-testbins rather than using wx? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16247 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABRT7MPXNCIJXW4YPMTBETRJEJMTANCNFSM4LOJUTNQ> .

[XVilka]

@x0urc3 can you add tests with dumps with and without tcache? And for the binary - it's probably better to include the glibc version in the name.

[x0urc3]

@x0urc3 can you add tests with dumps with and without tcache? And for the binary - it's probably better to include the glibc version in the name.

• ok. Test dmha/dmh for glibc x64 #16307 also test both with and without tcache. This PR will do the same
• Will create another PR to rename testbin
• Waiting for green on Test dmha/dmh for glibc x64 #16307 before proceeding

[XVilka]

@x0urc3 merged, please update.

[XVilka]

Well done - one missing thing left:

• Do not just replace the old value - use the new only for the newer glibc
• Determine the GLIBC version from runtime, maybe https://benohead.com/blog/2015/01/28/linux-check-glibc-version/ can be helpful
• Add test for the older glibc version too

[x0urc3]

Well done - one missing thing left:

• Do not just replace the old value - use the new only for the newer glibc

I don't understand what you mean here. Which value?

• Determine the GLIBC version from runtime, maybe https://benohead.com/blog/2015/01/28/linux-check-glibc-version/ can be helpful
• Add test for the older glibc version too

Can we do this in a different PR?

[XVilka]

@x0urc3 the value of SETI ("dbg.glibc.fc_offset", 0x00280, "First chunk offset from brk_start");

Determine the GLIBC version from runtime, maybe https://benohead.com/blog/2015/01/28/linux-check-glibc-version/ can be helpful
Add test for the older glibc version too

Can we do this in a different PR?

Yes, just please add a #ifdef for the GLIBC version meanwhile for the dbg.glibc.fc_offset, so it will not suddenly break for the users with older glibc versions installed.

[x0urc3]

• dbg.glibc.fc_offset does not depend on glibc version but host bit. The #ifdef to handle host bit (32/64) difference have already been in the code. This PR just reuse those
• There are few places where hardcoded dbg.glibc.fc_offset is still used and requires fixing