netdev - Re: [PATCH 10/19] bpf: Allow to store caller's ip as argument

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YL5iRT59TpOiUWe3@krava>
Date:   Mon, 7 Jun 2021 20:15:33 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Yonghong Song <yhs@...com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        "Steven Rostedt (VMware)" <rostedt@...dmis.org>,
        netdev@...r.kernel.org, bpf@...r.kernel.org,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>, Daniel Xu <dxu@...uu.xyz>,
        Viktor Malik <vmalik@...hat.com>
Subject: Re: [PATCH 10/19] bpf: Allow to store caller's ip as argument

On Sun, Jun 06, 2021 at 08:21:51PM -0700, Yonghong Song wrote:
> 
> 
> On 6/5/21 4:10 AM, Jiri Olsa wrote:
> > When we will have multiple functions attached to trampoline
> > we need to propagate the function's address to the bpf program.
> > 
> > Adding new BPF_TRAMP_F_IP_ARG flag to arch_prepare_bpf_trampoline
> > function that will store origin caller's address before function's
> > arguments.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >   arch/x86/net/bpf_jit_comp.c | 18 ++++++++++++++----
> >   include/linux/bpf.h         |  5 +++++
> >   2 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index b77e6bd78354..d2425c18272a 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -1951,7 +1951,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >   				void *orig_call)
> >   {
> >   	int ret, i, cnt = 0, nr_args = m->nr_args;
> > -	int stack_size = nr_args * 8;
> > +	int stack_size = nr_args * 8, ip_arg = 0;
> >   	struct bpf_tramp_progs *fentry = &tprogs[BPF_TRAMP_FENTRY];
> >   	struct bpf_tramp_progs *fexit = &tprogs[BPF_TRAMP_FEXIT];
> >   	struct bpf_tramp_progs *fmod_ret = &tprogs[BPF_TRAMP_MODIFY_RETURN];
> > @@ -1975,6 +1975,9 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >   		 */
> >   		orig_call += X86_PATCH_SIZE;
> > +	if (flags & BPF_TRAMP_F_IP_ARG)
> > +		stack_size += 8;
> > +
> >   	prog = image;
> >   	EMIT1(0x55);		 /* push rbp */
> > @@ -1982,7 +1985,14 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >   	EMIT4(0x48, 0x83, 0xEC, stack_size); /* sub rsp, stack_size */
> >   	EMIT1(0x53);		 /* push rbx */
> > -	save_regs(m, &prog, nr_args, stack_size);
> > +	if (flags & BPF_TRAMP_F_IP_ARG) {
> > +		emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, 8);
> > +		EMIT4(0x48, 0x83, 0xe8, X86_PATCH_SIZE); /* sub $X86_PATCH_SIZE,%rax*/
> 
> Could you explain what the above EMIT4 is for? I am not quite familiar with
> this piece of code and hence the question. Some comments here
> should help too.

it's there to generate the 'sub $X86_PATCH_SIZE,%rax' instruction
to get the real IP address of the traced function, and it's stored
to stack on the next line

I'll put more comments in there

jirka

> 
> > +		emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -stack_size);
> > +		ip_arg = 8;
> > +	}
> > +
> > +	save_regs(m, &prog, nr_args, stack_size - ip_arg);
> >   	if (flags & BPF_TRAMP_F_CALL_ORIG) {
> >   		/* arg1: mov rdi, im */
> > @@ -2011,7 +2021,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >   	}
> >   	if (flags & BPF_TRAMP_F_CALL_ORIG) {
> > -		restore_regs(m, &prog, nr_args, stack_size);
> > +		restore_regs(m, &prog, nr_args, stack_size - ip_arg);
> >   		if (flags & BPF_TRAMP_F_ORIG_STACK) {
> >   			emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, 8);
> > @@ -2052,7 +2062,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> >   		}
> >   	if (flags & BPF_TRAMP_F_RESTORE_REGS)
> > -		restore_regs(m, &prog, nr_args, stack_size);
> > +		restore_regs(m, &prog, nr_args, stack_size - ip_arg);
> >   	/* This needs to be done regardless. If there were fmod_ret programs,
> >   	 * the return value is only updated on the stack and still needs to be
> [...]
>