Skip to content

Conversation

Godones
Copy link
Member

@Godones Godones commented Jul 18, 2024

This commit extends the TrapFrme method to facilitate use by callback functions in kprobes. In order to find the function address based on the symbol, we added theaddr_from_symbol function.

In the x86 and riscv64 architecture related codes, we have added the handling of breakpoint exceptions and single-step exceptions.

This commit extends the TrapFrme method to facilitate use by callback
functions in kprobes. In order to find the function address based on
the symbol, we added the`addr_from_symbol` function.

In the x86 and riscv64 architecture related codes, we have added the
handling of breakpoint exceptions and single-step exceptions.
@github-actions github-actions bot added the enhancement New feature or request label Jul 18, 2024
@@ -54,7 +55,7 @@ uefi-raw = "=0.5.0"
paste = "=1.0.14"
slabmalloc = { path = "crates/rust-slabmalloc" }
log = "0.4.21"

kprobe = { git = "https://github.com/os-module/eebpf" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不应该出现没有指定rev的外部依赖

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@dragonosbot dragonosbot added O-riscv64 Target: riscv64 O-x86_64 Target: x86_64 labels Jul 18, 2024
Copy link
Member

@fslongjin fslongjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

从相关依赖库的代码量,以及内核可维护性的角度,没必要把kprobe的代码写到另一个仓库。这个是主线不能接受的~

@Godones
Copy link
Member Author

Godones commented Jul 18, 2024

我认为kprobe底层的实现是一个较为独立的模块,它不应该只局限于一个特定的内核当中。当然,我也可以把代码copy到主线当中。

@fslongjin
Copy link
Member

我认为kprobe底层的实现是一个较为独立的模块,它不应该只局限于一个特定的内核当中。当然,我也可以把代码copy到主线当中。

我的建议是copy到内核里面,因为这样对于DragonOS而言更容易维护,也更好的跟其他内核机制做结合(依赖库的话版本管理很麻烦,当要同时改内核和库的时候,开发效率较低)

@Godones
Copy link
Member Author

Godones commented Jul 18, 2024

好的

@fslongjin
Copy link
Member

这里为rv编译失败了,可以参考
https://docs.dragonos.org/zh-cn/latest/introduction/build_system.html#riscv64
来测试rv64的

1 similar comment
@fslongjin
Copy link
Member

这里为rv编译失败了,可以参考
https://docs.dragonos.org/zh-cn/latest/introduction/build_system.html#riscv64
来测试rv64的

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 18, 2024
Comment on lines 149 to 162
impl From<KprobeBuilder> for KprobeBasic {
fn from(value: KprobeBuilder) -> Self {
let fault_handler = value
.fault_handler
.unwrap_or_else(|| ProbeHandler::new(|_| {}));
KprobeBasic {
symbol: value.symbol.unwrap(),
symbol_addr: value.symbol_addr.unwrap(),
offset: value.offset.unwrap(),
pre_handler: value.pre_handler.unwrap(),
post_handler: value.post_handler.unwrap(),
fault_handler,
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这一块感觉改为实现TryFrom,或者Builder的new就要接受这几个变量并初始化,而不是在From的时候unwrap,因为这是pub可见的,可能有外部使用者并不知道这几个字段是要初始化才能Into KprobeBasic,就会导致panic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的

pub static DEBUG_KPROBE_LIST: SpinLock<BTreeMap<usize, Arc<Kprobe>>> =
SpinLock::new(BTreeMap::new());

pub fn kprobe_init() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个空函数的作用是?是还没写完还是什么

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为现在的实现中暂时不需要初始化其它内容,也许在未来需要,所以留着这个接口

}

#[cfg(feature = "kprobe_test")]
pub fn kprobe_test() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个测试会在哪里被运行吗,我好像没找到


#[inline(never)]
#[no_mangle]
pub fn detect_func(x: usize, y: usize) -> usize {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个应该不用pub可见吧

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该是的

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个为啥要no_mangle呀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为当前的函数地址查找实现是直接判断函数名称是否相等。如果不用no_mangle,rust的名称会被混淆。后面这里需要被重新实现。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为当前的函数地址查找实现是直接判断函数名称是否相等。如果不用no_mangle,rust的名称会被混淆。后面这里需要被重新实现。

确实这个函数名的问题,可以单独拎出来讨论一下。

Copy link
Member

@chiichen chiichen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

个人理解还有几个TODO,可以补充一下:

  • 直接计算指令长度,移除yaxpeax-x86/riscv64依赖项
  • 改进kernel/src/debug/kprobe/test.rs:7中查找函数地址的方式,去除no_mangle

@Godones
Copy link
Member Author

Godones commented Aug 2, 2024

@chiichen 这个计算指令长度的依赖依然是必要的。因为我们需要在注册kprobe的时候建立好映射关系,这也就需要提前知道这个指令长度的信息。在Linux源代码中,也有相关的解析指令长度的实现。我这里把riscv的依赖去掉了,因为其指令长度可以通过一些标志位就知道。

@chiichen chiichen requested a review from fslongjin August 8, 2024 15:00
@chiichen
Copy link
Member

chiichen commented Aug 8, 2024

LGTM

@Godones
Copy link
Member Author

Godones commented Aug 14, 2024

@fslongjin 请问这个pr还有什么需要解决的吗?

@chiichen chiichen changed the base branch from master to feat-ebpf August 19, 2024 08:04
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Aug 19, 2024
@chiichen chiichen merged commit d9b12fe into DragonOS-Community:feat-ebpf Aug 19, 2024
8 checks passed
@Godones Godones deleted the kprobe branch July 26, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request O-riscv64 Target: riscv64 O-x86_64 Target: x86_64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants