Hi Alexander, >> - Per-task buffer: task->kcov_df_area with atomic xadd reservation > > I don't understand this line... > >> - Recursion-safe: notrace __no_sanitize_coverage noinline >> - ERR_PTR aware: skips struct expansion for error pointers > > ... and this.
I updated this text at v2 patch. >> >> The callbacks (__sanitizer_cov_trace_args/ret) are inserted by the >> compiler when -fsanitize-coverage=dataflow-args,dataflow-ret is used. >> The Kconfig options depend on cc-option to verify compiler support. >> >> Buffer format (TLV records, all u64): >> area[0]: atomic word count >> [pos+0]: type_and_seq (0xE=entry, 0xF=return in upper 4 bits) >> [pos+1]: PC >> [pos+2]: meta (arg_idx | arg_size | ptr) >> [pos+3..N]: field values read via copy_from_kernel_nofault() >> >> This is completely independent from legacy /sys/kernel/debug/kcov. >> Existing users (syzkaller, oss-fuzz) are unaffected. > > Does oss-fuzz even use kcov? Also, I removed this text at v2 patch. I mistakenly confused it with another usage of KCOV with a other fuzzer. https://security.googleblog.com/2024/06/hacking-for-defenders-approaches-to.html >> >> Signed-off-by: Yunseong Kim <[email protected]> >> --- >> include/linux/sched.h | 8 ++ >> kernel/kcov.c | 291 >> ++++++++++++++++++++++++++++++++++++++++++++++++++ >> lib/Kconfig.debug | 22 ++++ >> 3 files changed, 321 insertions(+) >> >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index c4433c185ad8..03be4b495f70 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -1533,6 +1533,14 @@ struct task_struct { >> /* KCOV sequence number: */ >> int kcov_sequence; >> >> + /* KCOV dataflow per-task sequence counter for TLV records: */ >> + u32 kcov_dataflow_seq; >> + >> + /* KCOV dataflow: separate buffer for trace-args/trace-ret */ >> + unsigned int kcov_df_size; >> + void *kcov_df_area; >> + bool kcov_df_enabled; >> + >> /* Collect coverage from softirq context: */ >> unsigned int kcov_softirq; >> #endif >> diff --git a/kernel/kcov.c b/kernel/kcov.c >> index 1df373fb562b..d3c9c0efe961 100644 >> --- a/kernel/kcov.c >> +++ b/kernel/kcov.c >> @@ -353,6 +353,288 @@ void notrace __sanitizer_cov_trace_switch(kcov_u64 >> val, void *arg) >> EXPORT_SYMBOL(__sanitizer_cov_trace_switch); >> #endif /* ifdef CONFIG_KCOV_ENABLE_COMPARISONS */ >> >> +#if defined(CONFIG_KCOV_DATAFLOW_ARGS) || defined(CONFIG_KCOV_DATAFLOW_RET) >> +/* >> + * KCOV Dataflow: /sys/kernel/debug/kcov_dataflow >> + * >> + * Completely separate from legacy /sys/kernel/debug/kcov. > > Since this code is completely separate, could it be put into a separate file? > I think kcov.c is too big already. Thank you again for your guide, I updated it at v2. >> + * Own buffer, own ioctl, own mmap. No printk — buffer only. > > Can you please not use these long dashes in C code? I removed all a the v2. >> +/* >> + * Core write function — no printk, no locks, just atomic buffer write. > > I think it's okay to omit what this function is not doing. > > >> + >> + /* Atomic reservation */ >> + pos = 1 + xadd((unsigned long *)&area[0], record_len); >> + if (unlikely(pos + record_len > max_pos)) { >> + xadd((unsigned long *)&area[0], -(long)record_len); >> + return; >> + } > > Have you tried compiling this code on ARM64? > I am pretty sure they don't have xadd(), so it won't work. > But why do we need an atomic increment here at all? write_comp_data() > performs the same job, and does not need it. > Or am I missing something? Thank you again for pointing out. After updating to the READ_ONCE/WRITE_ONCE atomic pattern, Testing results based on v2 on arm64 for the Rust for Linux kernel module(eight_struct_args_rust) are as follows: do_el0_svc({0xffffffffffffff9c, 0xffffffffffffff9c, 0xffffffff, 0x0, 0x0, 0x0}) invoke_syscall({0xffffffffffffff9c, 0xffffffffffffff9c, 0x38, 0x0, 0x0, 0x0}, 0x38) __arm64_sys_openat({0xffffffffffffff9c, 0xffffffffffffff9c, 0x38, 0x0, 0x0, 0x0}) ksys_write(0xffff9a031231, 0x1) fdget_pos(0x4) 0xffff000004421cc0 = fdget_pos() 0x0 = vfs_write() vfs_write(0xffff9a031231, 0x1, 0x0) 0x0 = _RNvCsdfZGIOKgjaD_22eight_struct_args_rust13write_handler [eight_struct_args_rust]() _RNvCsdfZGIOKgjaD_22eight_struct_args_rust13write_handler [eight_struct_args_rust](0xffff9a031231, 0x1, 0x0) rsf_1 [eight_struct_args_rust](0x11) 0x11 = rsf_1 [eight_struct_args_rust]() rsf_2 [eight_struct_args_rust](0x11, {0x11, 0x22}) 0x33 = rsf_2 [eight_struct_args_rust]() rsf_4 [eight_struct_args_rust](0x11, {0x11, 0x22}, {0x11, 0x22, 0x33}, {0x11, 0x22, 0x33, 0x44}) 0xaa = rsf_4 [eight_struct_args_rust]() ... Latest test results from Github CI: https://github.com/yskzalloc/kcov-dataflow/actions/runs/27397351811/job/80967927283 Best regards, Yunseong
