Linux rise in rizin: SLUB dumping, Kernel build configuration #4306

rockrid3r · 2024-02-28T07:19:12Z

DO NOT MERGE IT'S STILL UNTESTED

There is no legitimate way to test the functionality now. I will tell you later why exactly.
Firstly the rizin is slow and OOMs when using large ELF with debug symbols (vmlinux) #4385 should improve

Addresses #4257

SLUB dumping

Dumping of lockless freelist
Dumping of other freelists: regular(locking), partial, node freelist
Handle different kernel version & .config configuration
~~Add integration tests~~(is not suitable)

Vmlinux(TODO: Separate PR)

~~Add support to rebase kernel binary (KASLR)~~
~~Add unit tests~~

Kernel version handling

Use 6.7 kernel as default kernel version
Add flag to change kernel version: -e linux.version=5.6.11
~~Add rizin shell command to change kernel version.~~
Add tests

Kernel configuration handling

Add defconfig configuration file. It will contain all the default flags used while building kernel.
It will be the presumed kernel configuration if user does not supply more. (User should set it up in ~/.config/rizin/defconfig)
~~Add CLI flag to support user-supplied kernel build configuration flags. For example, this:~~

rizin vmlinux --add-vmlinux-flag CONFIG_FREELIST_RANDOM=y

~~should open~~ vmlinux ~~file and handle the fact that kernel was built with configuration~~ defconfig + CONFIG_FREELIST_RANDOM=y

~~Add rizin shell command user-supplied configuration:~~ > set_config CONFIG_FREELIST_RANDOM=y ~~(same as before, but in shell)~~
Accept user-supplied config file. For example, this:

rizin vmlinux -e linux.config=.config

implies that rizing will use .config instead of defconfig.

Add unit tests ~~& integration tests~~

Basic checklist

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

rockrid3r · 2024-02-28T07:22:56Z

The PR is very far from over.

Buggy.
Has hardcoded member offsets.
Does not process kernel build configuration .config
Does not reflect Linux Kernel version
etc.

Will add todo later.
All comments are appreciated.

ret2libc · 2024-02-28T07:58:33Z

librz/core/cmd/cmd_linux_heap_slub.c

+#define call_handler(fun, ...) \
+	{ \
+		if (core->rasm->bits == 64) { \
+			return fun##_64(core, ##__VA_ARGS__); \
+		} else { \
+			return fun##_32(core, ##__VA_ARGS__); \
+		} \
+	}
+
+RZ_IPI RzCmdStatus rz_cmd_debug_slub_dump_freelist_handler(RzCore *core, int argc, const char **argv, RzCmdStateOutput *output_state) {
+	call_handler(rz_cmd_debug_slub_dump_freelist_handler, argc, argv, output_state);
+}


Maybe I'm wrong but can't we just do the following?

Suggested change

#define call_handler(fun, ...) \

{ \

if (core->rasm->bits == 64) { \

return fun##_64(core, ##__VA_ARGS__); \

} else { \

return fun##_32(core, ##__VA_ARGS__); \

} \

}

RZ_IPI RzCmdStatus rz_cmd_debug_slub_dump_freelist_handler(RzCore *core, int argc, const char **argv, RzCmdStateOutput *output_state) {

call_handler(rz_cmd_debug_slub_dump_freelist_handler, argc, argv, output_state);

}

RZ_IPI RzCmdStatus rz_cmd_debug_slub_dump_freelist_handler(RzCore *core, int argc, const char **argv, RzCmdStateOutput *output_state) {

if (core->rasm->bits == 64) {

return rz_cmd_debug_slub_dump_freelist_handler_64(core, argc, argv, output_state);

} else {

return rz_cmd_debug_slub_dump_freelist_handler_32(core, argc, argv, output_state);

}

}

I will add more functions. Macro will come in handy.
See librz/core/cmd/cmd_linux_heap_glibc.c

ret2libc · 2024-02-28T07:59:11Z

librz/analysis/dwarf_process.c

are these dwarf changes related to the linux heap changes?

Nope, all the dwarf changes are for the sake of debugging the functionality.
I will remove them as soon as I am finished.

It speeds up rizin startup: takes 1 minute instead of 10+.

If you can, please split them in a separate PR that we can merge sooner then! It would be great to have such an improvement :)

But only if it's general enough, currently it just skips everything except specific file. cc @imbillow

@ret2libc of course it's not a problem for me. But you probably don't want to have it.
It's not an optimization. @XVilka is right, it just skips everything.

ret2libc · 2024-02-28T08:00:34Z

Thanks @rockrid3r ! Please also add a short description of what you are trying to achieve so people reviewing know immediately and a better title for the PR

librz/include/rz_util/rz_num.h

rockrid3r · 2024-02-28T13:41:50Z

Thanks @rockrid3r ! Please also add a short description of what you are trying to achieve so people reviewing know immediately and a better title for the PR

My bad. I've pinned the issue link and changed PR title.

rockrid3r · 2024-03-06T04:48:19Z

Currently PR is in much better state. It's almost ready for merge.

Code is clean. No debug output.
Uses struct offsets from dwarf (not hardcoded)
Does processes kernel configuration
Does processes kernel version

Only tests are left.

wargio · 2024-03-12T00:29:14Z

librz/core/cconfig.c

+	RZ_LOG_INFO("Parsing config file '%s'...\n", vmlinux_config);
 	vmlinux_parse_apply_config_file(vmlinux_config, core->analysis->vmlinux_config->config_tbl);


maybe before parsing, you should check if the string is empty and if the file exists.

Hello. I am verifying it inside the function vmlinux_parse_apply_config_file

wargio · 2024-03-12T00:29:32Z

librz/core/cconfig.c


 	if (RZ_STR_ISNOTEMPTY(apply_config_file)) {
-		printf("Parsing apply_config file '%s'\n", apply_config_file);
+		RZ_LOG_INFO("Parsingconfig file '%s'\n", apply_config_file);


Suggested change

RZ_LOG_INFO("Parsingconfig file '%s'\n", apply_config_file);

RZ_LOG_INFO("Parsing config file '%s'\n", apply_config_file);

fixed, thanks

rockrid3r · 2024-03-21T11:43:06Z

I've added basic tests for version handler and configuration handler

XVilka · 2024-03-21T14:09:57Z

I've added basic tests for version handler and configuration handler

There are merge conflicts, please rebase and solve them.

XVilka

@wargio @thestr4ng3r @ret2libc @Rot127 please take a look too, mostly at where things are located architecturally.

XVilka · 2024-03-21T14:10:29Z

test/unit/test_vmlinux.c

+#include <stdbool.h>
+
+static bool test_vmlinux_vercmp(void) {
+    unsigned long v1[3] = {6, 7, 1};


Broken indentation

ran clang-format

XVilka · 2024-03-21T14:11:21Z

librz/include/vmlinux.h

@@ -0,0 +1,29 @@
+#pragma once


SPDX

Doxygen comments for added structures and the purpose of this file

please don't use #pragma once but use proper #ifndef XXX #define XXX also add the c++ guards.

added doxygen & spdx. usnig #ifndef now.

@wargio could you please elaborate more on c++ guards? what should be added?

XVilka · 2024-03-21T14:12:38Z

librz/include/rz_analysis.h

@@ -535,6 +537,7 @@ typedef struct rz_analysis_t {
 	RzAnalysisDebugInfo *debug_info; ///< store all debug info parsed from DWARF, etc..
 	ut64 cmpval; ///< last compare value for jump table.
 	ut64 lea_jmptbl_ip; ///< jump table x86 lea ip
+	RzVmlinuxConfig* vmlinux_config;


I am not particularly happy about adding the vmlinux config directly in the RzAnalysis; honestly, it looks out of place. @wargio @ret2libc @Rot127 @thestr4ng3r any ideas?

The proper way would be to have it in it's own plugin IMHO. Because the logic is very much contained. But it is good how it is currently. Wouldn't know a place where the config fits else.
Added it #4334 as code which should be move to it's own plugin after refactoring. Please remove it if you disagree.

i wonder if this should be a bin+io plugin with, like we have for dmp

XVilka · 2024-03-21T14:13:08Z

librz/core/vmlinux.c

+
+static void add_config(RzVmlinuxConfigTable* config_tbl, char* config_name, char* config_value);
+
+RZ_API bool vmlinux_parse_apply_config_file(const char* config_filepath, RzVmlinuxConfigTable* config_tbl) {


Doxygen for every new or changed RZ_API function except obvious ones like _free()

added doxygen.

XVilka · 2024-03-21T14:13:30Z

librz/core/vmlinux.c

+static void add_config(RzVmlinuxConfigTable* config_tbl, char* config_name, char* config_value);
+
+RZ_API bool vmlinux_parse_apply_config_file(const char* config_filepath, RzVmlinuxConfigTable* config_tbl) {
+    rz_return_val_if_fail(config_filepath && config_tbl, false);


If you add assert, please also add attributes like RZ_NONNULL

also add rz_ prefix for any new functionality that is RZ_API

added both prefix and NONNULL

XVilka · 2024-03-21T14:17:36Z

librz/core/vmlinux.c

+    free(vmlinux_config);
+}
+
+RZ_API void rz_vmlinux_config_table_free(RzVmlinuxConfigTable* config_tbl) {


RZ_NULLABLE

XVilka · 2024-03-21T14:18:05Z

librz/core/linux_heap_slub.h

@@ -0,0 +1,33 @@
+#include <rz_core.h>
+


SPDX and Doxygen for file description

XVilka · 2024-03-21T14:18:16Z

librz/core/linux_heap_slub.c

@@ -0,0 +1,1060 @@
+#include <rz_core.h>


XVilka · 2024-03-21T14:18:52Z

librz/core/cmd_descs/cmd_slub.yaml

@@ -0,0 +1,55 @@
+# SPDX-FileCopyrightText: 2021 RizinOrg <[email protected]>


XVilka · 2024-03-21T14:19:17Z

librz/core/cmd/cmd_linux_heap_slub.c

@@ -0,0 +1,34 @@
+// SPDX-FileCopyrightText: 2021 Pulak Malhotra <[email protected]>


Wrong SPDX, put yours

fixed, thanks.

Rot127 · 2024-03-21T14:33:15Z

test/unit/test_vmlinux.c

@@ -0,0 +1,50 @@
+// SPDX-FileCopyrightText: 2017 Fangrui Song <[email protected]>


Please put your copyright here.

fixed, thanks.

Rot127 · 2024-03-21T14:33:48Z

test/unit/test_vmlinux.c

+
+	size_t config_size = sizeof(config_lines) / sizeof(config_lines[0]);
+	for (size_t i = 0; i < config_size; ++i) {
+		// eprintf("%p, %p\n", config_lines[i], config_tbl);


Please, remove debug code when done.

fixed, thanks.

Rot127

Looked briefly. And in general LGTM. Please though, resolve comments from @XVilka. Especially the doxygen is important.
If you find the time it would be nice, if you could quickly run the unittest with valgrind --leak-check=full.

2. Bug fixes. 3. Refactor.

rockrid3r requested review from ret2libc, thestr4ng3r, wargio, XVilka and kazarmy as code owners February 28, 2024 07:19

github-actions bot added RzAnalysis API RzCore DWARF RzUtil labels Feb 28, 2024

ret2libc reviewed Feb 28, 2024

View reviewed changes

wargio reviewed Feb 28, 2024

View reviewed changes

librz/include/rz_util/rz_num.h Outdated Show resolved Hide resolved

rockrid3r changed the title ~~basic functionality~~ SLUB dumping: basic functionality Feb 28, 2024

rockrid3r changed the title ~~SLUB dumping: basic functionality~~ Linux rise in rizin: SLUB dumping, Kernel build configuration Feb 28, 2024

github-actions bot added API and removed API labels Feb 28, 2024

rockrid3r requested review from ret2libc and wargio March 1, 2024 06:46

github-actions bot added the rz-test label Mar 5, 2024

github-actions bot removed RzAnalysis DWARF labels Mar 11, 2024

wargio reviewed Mar 12, 2024

View reviewed changes

rockrid3r requested a review from wargio March 21, 2024 11:41

XVilka requested changes Mar 21, 2024

View reviewed changes

Rot127 reviewed Mar 21, 2024

View reviewed changes

Rot127 mentioned this pull request Mar 21, 2024

[RzArch] Progress tracker #4334

Open

73 tasks

rockrid3r and others added 13 commits March 23, 2024 15:56

Linux kmalloc cache dump support

78746f6

replace GH with GH_

97b6f4e

replace GH with GH_ and use rz_swap utility from rz_endian.h

43d9a2c

add rz_str_split_by_first_dupstr

48dee57

Added kernel configuration & version parsing

9156f5f

1. Added dump of regular freelist(untested).

40d9c90

2. Bug fixes. 3. Refactor.

dump for regular, partial, node freelists

ff5db0c

removed debug output

e6904ad

added len info for printed freelists

ce4b276

added error handling

d8344bb

fixed bug

c826566

Added tests for vmlinux version & config

bc33917

implementing required fixes

62ac804

rockrid3r force-pushed the slub-dump-generic-caches branch from a637b5c to 62ac804 Compare March 23, 2024 16:51

github-actions bot added the RzAnalysis label Mar 23, 2024

rockrid3r requested review from XVilka and Rot127 March 23, 2024 17:18

rockrid3r added 2 commits March 23, 2024 20:25

clang-format

7562f0c

fixed linter + clang-format

eefdf34

		RZ_LOG_INFO("Parsing config file '%s'...\n", vmlinux_config);
		vmlinux_parse_apply_config_file(vmlinux_config, core->analysis->vmlinux_config->config_tbl);

	RZ_LOG_INFO("Parsingconfig file '%s'\n", apply_config_file);
	RZ_LOG_INFO("Parsing config file '%s'\n", apply_config_file);


		static void add_config(RzVmlinuxConfigTable* config_tbl, char* config_name, char* config_value);

		RZ_API bool vmlinux_parse_apply_config_file(const char* config_filepath, RzVmlinuxConfigTable* config_tbl) {

		@@ -0,0 +1,55 @@
		# SPDX-FileCopyrightText: 2021 RizinOrg <[email protected]>

		@@ -0,0 +1,34 @@
		// SPDX-FileCopyrightText: 2021 Pulak Malhotra <[email protected]>

		@@ -0,0 +1,50 @@
		// SPDX-FileCopyrightText: 2017 Fangrui Song <[email protected]>

Linux rise in rizin: SLUB dumping, Kernel build configuration #4306

Are you sure you want to change the base?

Linux rise in rizin: SLUB dumping, Kernel build configuration #4306

Conversation

rockrid3r commented Feb 28, 2024 • edited Loading

DO NOT MERGE IT'S STILL UNTESTED

SLUB dumping

Vmlinux(TODO: Separate PR)

Kernel version handling

Kernel configuration handling

rockrid3r commented Feb 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rockrid3r Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ret2libc commented Feb 28, 2024

rockrid3r commented Feb 28, 2024

rockrid3r commented Mar 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rockrid3r commented Mar 21, 2024

XVilka commented Mar 21, 2024

XVilka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rot127 left a comment

Choose a reason for hiding this comment

rockrid3r commented Feb 28, 2024 •

edited

Loading

rockrid3r Feb 28, 2024 •

edited

Loading