Re: [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64

[Yonghong Song <yhs@xxxxxx>]

On 1/6/22 6:38 PM, Kenta.Tada@xxxxxxxx wrote:
> Currently, rcx is read as the fourth parameter of syscall on x86_64.
> But x86_64 Linux System Call convention uses r10 actually.
> This commit adds the wrapper for users who want to access to
> syscall params to analyze the user space.
>
> Changelog:
> ----------
> v1 -> v2:
> - Rebase to current bpf-next
> https://lore.kernel.org/bpf/20211222213924.1869758-1-andrii@xxxxxxxxxx/
>
> Signed-off-by: Kenta Tada <Kenta.Tada@xxxxxxxx>
> ---
>   tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index 90f56b0f585f..4c3df8c122a4 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -70,6 +70,7 @@
>   #define __PT_PARM2_REG si
>   #define __PT_PARM3_REG dx
>   #define __PT_PARM4_REG cx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
>   #define __PT_PARM5_REG r8
>   #define __PT_RET_REG sp
>   #define __PT_FP_REG bp
> @@ -99,6 +100,7 @@
>   #define __PT_PARM2_REG rsi
>   #define __PT_PARM3_REG rdx
>   #define __PT_PARM4_REG rcx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
>   #define __PT_PARM5_REG r8
>   #define __PT_RET_REG rsp
>   #define __PT_FP_REG rbp
> @@ -226,6 +228,7 @@ struct pt_regs;
>   #define PT_REGS_PARM2(x) (__PT_REGS_CAST(x)->__PT_PARM2_REG)
>   #define PT_REGS_PARM3(x) (__PT_REGS_CAST(x)->__PT_PARM3_REG)
>   #define PT_REGS_PARM4(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG)
> +#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG_SYSCALL)
>   #define PT_REGS_PARM5(x) (__PT_REGS_CAST(x)->__PT_PARM5_REG)
>   #define PT_REGS_RET(x) (__PT_REGS_CAST(x)->__PT_RET_REG)
>   #define PT_REGS_FP(x) (__PT_REGS_CAST(x)->__PT_FP_REG)
> @@ -237,6 +240,7 @@ struct pt_regs;
>   #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM2_REG)
>   #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM3_REG)
>   #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG)
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL)
>   #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM5_REG)
>   #define PT_REGS_RET_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_RET_REG)
>   #define PT_REGS_FP_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_FP_REG)
> @@ -292,6 +296,22 @@ struct pt_regs;
>   
>   #endif /* defined(bpf_target_defined) */
>   
> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> +#ifndef PT_REGS_PARM4_SYSCALL
> +#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
> +#endif
> +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> +
> +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> +#ifndef PT_REGS_PARM4_CORE_SYSCALL
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) PT_REGS_PARM4_CORE(x)
> +#endif
> +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)

Since you proposed the patch, you probably hit the issue. Please
add a selftest so the macro esp. PT_REGS_PARM4_SYSCALL or
PT_REGS_PARM4_CORE_SYSCALL is exercised. Thanks.

> +
>   #ifndef ___bpf_concat
>   #define ___bpf_concat(a, b) a ## b
>   #endif