netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Jiri Olsa <jolsa@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andriin@fb.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>
Subject: Re: [PATCH] bpf: Forbid trampoline attach for functions with variable arguments
Date: Fri, 7 May 2021 10:13:09 +0200	[thread overview]
Message-ID: <YJT2lbKUCRo90MQV@krava> (raw)
In-Reply-To: <c21b54a6-8d6e-f76d-e6c1-95abd8544d9d@iogearbox.net>

On Fri, May 07, 2021 at 01:31:54AM +0200, Daniel Borkmann wrote:
> On 5/5/21 8:45 PM, Andrii Nakryiko wrote:
> > On Wed, May 5, 2021 at 6:42 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > 
> > > We can't currently allow to attach functions with variable arguments.
> > > The problem is that we should save all the registers for arguments,
> > > which is probably doable, but if caller uses more than 6 arguments,
> > > we need stack data, which will be wrong, because of the extra stack
> > > frame we do in bpf trampoline, so we could crash.
> > > 
> > > Also currently there's malformed trampoline code generated for such
> > > functions at the moment as described in:
> > >    https://lore.kernel.org/bpf/20210429212834.82621-1-jolsa@kernel.org/
> > > 
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > 
> > LGTM.
> > 
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > 
> > >   kernel/bpf/btf.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index 0600ed325fa0..161511bb3e51 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -5206,6 +5206,13 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
> > >          m->ret_size = ret;
> > > 
> > >          for (i = 0; i < nargs; i++) {
> > > +               if (i == nargs - 1 && args[i].type == 0) {
> > > +                       bpf_log(log,
> > > +                               "The function %s with variable args is unsupported.\n",
> > > +                               tname);
> > > +                       return -EINVAL;
> > > +
> 
> (Jiri, fyi, I removed this extra newline while applying. Please scan for such
> things before submitting.)

sorry, will do

jirka


  reply	other threads:[~2021-05-07  8:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 13:25 [PATCH] bpf: Forbid trampoline attach for functions with variable arguments Jiri Olsa
2021-05-05 18:45 ` Andrii Nakryiko
2021-05-06 23:31   ` Daniel Borkmann
2021-05-07  8:13     ` Jiri Olsa [this message]
2021-05-06 23:30 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJT2lbKUCRo90MQV@krava \
    --to=jolsa@redhat.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).