Re: [PATCH bpf-next v4 2/4] bpf: Prevent extending tail callee prog with freplace prog

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Fri, Oct 4, 2024 at 1:52 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Fri, 2024-10-04 at 12:33 -0700, Alexei Starovoitov wrote:
>
> [...]
>
> > btw the whole thing can be done with a single atomic64_t:
> > - set it to 1 at the start then
> >
> > - prog_fd_array_get_ptr() will do
> > atomic64_inc_not_zero
> >
> > - prog_fd_array_put_ptr() will do
> > atomic64_add_unless(,-1, 1)
> >
> > - freplace attach will do
> > cmpxchg(,1,0)
> >
> > so 1 - initial state
> > 2,3,.. - prog in prog_array
> > 0 - prog was extended.
> >
> > If == 0 -> cannot add to prog_array
> > if > 1 -> cannot freplace.
>
> I think this should work, because we no longer need to jungle two values.
> I kinda like it.

It's a bit too clever.

With mutex it's much easier to reason about:

struct bpf_prog_aux {
   mutex ext_mutex;
   bool is_extended;
   u64 prog_array_member_cnt;
};

freplace link on tgt_prog:
guard(mutex)(&aux->ext_mutex);
if (aux->prog_array_member_cnt) {
  // reject freplace
} else {
  aux->is_extended = true;
}

freplace unlink:
guard(mutex)(&aux->ext_mutex);
aux->is_extended = false;

and similar in prog_fd_array_get_ptr():
guard(mutex)(&aux->ext_mutex);
if (aux->is_extended) {
   // reject adding to prog_array
} else {
  aux->prog_array_member_cnt++;
}

in prog_fd_array_put_ptr():
guard(mutex)(&aux->ext_mutex);
aux->prog_array_member_cnt--;