netdev - Re: [PATCH v4 bpf-next 1/4] bpf: Factor out bpf_link_get_by

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: <CAPhsuW51PCaza9yu-SRVvjaAOU3_fxCEsY9Jjq8zzCFwS8oDpg@mail.gmail.com>
Date:   Wed, 29 Jul 2020 15:25:44 -0700
From:   Song Liu <song@...nel.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v4 bpf-next 1/4] bpf: Factor out bpf_link_get_by_id() helper.

On Fri, Jul 24, 2020 at 1:39 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> From: Alexei Starovoitov <ast@...nel.org>
>
> Refactor the code a bit to extract bpf_link_get_by_id() helper.

The commit log doesn't match the code: bpf_link_get_by_id() vs.
bpf_link_by_id().

> It's similar to existing bpf_prog_by_id().
>
> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
> Acked-by: Andrii Nakryiko <andriin@...com>

Other than that,

Acked-by: Song Liu <songliubraving@...com>

> ---
>  include/linux/bpf.h  |  1 +
>  kernel/bpf/syscall.c | 46 +++++++++++++++++++++++++++-----------------
>  2 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 8357be349133..d5c4e2cc24a0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1355,6 +1355,7 @@ int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog,
>                          struct btf *btf, const struct btf_type *t);
>
>  struct bpf_prog *bpf_prog_by_id(u32 id);
> +struct bpf_link *bpf_link_by_id(u32 id);
>
>  const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id);
>  #else /* !CONFIG_BPF_SYSCALL */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ee290b1f2d9e..42eb0d622980 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3984,40 +3984,50 @@ static int link_update(union bpf_attr *attr)
>         return ret;
>  }
>
> -static int bpf_link_inc_not_zero(struct bpf_link *link)
> +static struct bpf_link *bpf_link_inc_not_zero(struct bpf_link *link)
>  {
> -       return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? 0 : -ENOENT;
> +       return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? link : ERR_PTR(-ENOENT);
>  }
>
> -#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id
> -
> -static int bpf_link_get_fd_by_id(const union bpf_attr *attr)
> +struct bpf_link *bpf_link_by_id(u32 id)
>  {
>         struct bpf_link *link;
> -       u32 id = attr->link_id;
> -       int fd, err;
>
> -       if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID))
> -               return -EINVAL;
> -
> -       if (!capable(CAP_SYS_ADMIN))
> -               return -EPERM;
> +       if (!id)
> +               return ERR_PTR(-ENOENT);
>
>         spin_lock_bh(&link_idr_lock);
> -       link = idr_find(&link_idr, id);
>         /* before link is "settled", ID is 0, pretend it doesn't exist yet */
> +       link = idr_find(&link_idr, id);
>         if (link) {
>                 if (link->id)
> -                       err = bpf_link_inc_not_zero(link);
> +                       link = bpf_link_inc_not_zero(link);
>                 else
> -                       err = -EAGAIN;
> +                       link = ERR_PTR(-EAGAIN);
>         } else {
> -               err = -ENOENT;
> +               link = ERR_PTR(-ENOENT);
>         }
>         spin_unlock_bh(&link_idr_lock);
> +       return link;
> +}
>
> -       if (err)
> -               return err;
> +#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id
> +
> +static int bpf_link_get_fd_by_id(const union bpf_attr *attr)
> +{
> +       struct bpf_link *link;
> +       u32 id = attr->link_id;
> +       int fd;
> +
> +       if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID))
> +               return -EINVAL;
> +
> +       if (!capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       link = bpf_link_by_id(id);
> +       if (IS_ERR(link))
> +               return PTR_ERR(link);
>
>         fd = bpf_link_new_fd(link);
>         if (fd < 0)
> --
> 2.23.0
>