netdev - Re: bpf indirect calls

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] [day] [month] [year] [list]
Message-ID: <ZTEg_lJ9QE3VvLP5@google.com>
Date: Thu, 19 Oct 2023 12:28:46 +0000
From: Matt Bobrowski <mattbobrowski@...gle.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Toke Høiland-Jørgensen <toke@...hat.com>,
	Daniel Borkmann <daniel@...earbox.net>,
	Alexei Starovoitov <ast@...nel.org>,
	Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>,
	Yonghong Song <yhs@...com>, Marek Majkowski <marek@...udflare.com>,
	Lorenz Bauer <lmb@...udflare.com>,
	Alan Maguire <alan.maguire@...cle.com>,
	Jesper Dangaard Brouer <brouer@...hat.com>,
	David Miller <davem@...emloft.net>,
	Network Development <netdev@...r.kernel.org>,
	bpf <bpf@...r.kernel.org>
Subject: Re: bpf indirect calls

On Fri, Oct 06, 2023 at 11:49:37AM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 6, 2023 at 2:36 AM Matt Bobrowski <mattbobrowski@...gle.com> wrote:
> > On Fri, Sep 29, 2023 at 02:06:10PM -0700, Alexei Starovoitov wrote:
> > > On Wed, Sep 27, 2023 at 6:27 AM Matt Bobrowski <mattbobrowski@...gle.com> wrote:
> > > > static void testing(void) {
> > > >   bpf_printk("testing");
> > > > }
> > > >
> > > > struct iter_ctx {
> > > >   void (*f) (void);
> > > > };
> > > > static u64 iter_callback(struct bpf_map *map, u32 *key,
> > > >                          u64 *value, struct iter_ctx *ctx) {
> > > >   if (ctx->f) {
> > > >     ctx->f();
> > > >   }
> > > >   return 0;
> > > > }
> > > >
> > > > SEC("lsm.s/file_open")
> > > > int BPF_PROG(file_open, struct file *file)
> > > > {
> > > >   struct iter_ctx iter_ctx = {
> > > >     .f = testing,
> > > >   };
> > > >   bpf_for_each_map_elem(&map, iter_callback, &iter_ctx, 0);
> > > >   return 0;
> > > > }
> > > > ```
> > > ...
> > > > The fundamental difference between the two call instructions if I'm
> > > > not mistaken is that one attempts to perform a call using an immediate
> > > > value as its source operand, whereas the other attempts to perform a
> > > > call using a source register as its source operand. AFAIU, the latter
> > > > is not currently permitted by the BPF verifier. Is that right?
> > >
> > > Correct. Indirect calls via 'callx' instruction are not supported yet.
> > > Please use bpf_tail_call() as a workaround for now.
> >
> > Noted.
> >
> > > Over the years the verifier became progressively smarter and maybe
> > > now is a good time to support true indirect calls.
> >
> > This is something that I wouldn't mind exploring myself as a little
> > research/contribution project. Would you object to me taking this on?
> > I feel as though this would give me an opportunity to develop a better
> > understanding when it comes to the internals of the BPF subsystem.
> 
> Please go ahead, but let's get to the bottom of your concern first.
> See below.

OK, sure.

> > > For certain cases like your example above it's relatively easy to
> > > add such support, but before we do that please describe the full use
> > > case that you wanted to implement with indirect calls.
> >
> > For the specific example I provided above, using indirect calls was an
> > approach that I considered using within one of our BPF programs in
> > order to work around this [0] specific BPF verifier shortcoming. For
> > the workaround, I needed to implement 2 BPF programs that more or less
> > done the same thing using the same set of routines, but differed ever
> > so slightly for one particular routine. The way I envisioned
> > controlling that one small difference between the 2 BPF programs is by
> > supplying in different function pointers within the iteration context
> > passed to bpf_for_each_map_elem(),
> 
> Early in that [0] link you were asking about kfunc detection and
> the issue was that it's not backported to older kernels.
> Here you're proposing a totally new feature of indirect calls which
> is a magnitude bigger than kfunc detection.
> Highly unlikely it will be backported to older kernels.
> For google kernels you can backport anything you want, of course.

Well, that holds true most of the time, but it also depends on the
context you're talking about of course. As you can also imagine, some
areas are a little more receptive at applying a specific backport then
others.

> So backport of kfunc detection would have been enough and
> you wouldn't need indirect calls ?

Yes, I agree that backporting a specific patch would fundamentally
address the specific problem that I ran into here, but we didn't take
that route so I was left scrambling looking for alternate
approaches. Regardless of the decision made in this specific scenario
on whether to backport or not, it doesn't go to say that we shouldn't
look at implementing indirect call support, right?

/M