On Fri, Dec 13, 2024 at 3:35 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 12/7/24 9:37 AM, Jason Xing wrote:
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 6625b3f563a4..f7e9f88e09b1 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5214,6 +5214,24 @@ static const struct bpf_func_proto bpf_get_socket_uid_proto = {
> > .arg1_type = ARG_PTR_TO_CTX,
> > };
> >
> > +static int sk_bpf_set_cb_flags(struct sock *sk, sockptr_t optval, bool getopt)
>
> It is confusing to take a sockptr_t argument. It is called by the kernel bpf
> prog only. It must be from the kernel memory. Directly pass the "int
> sk_bpf_cb_flags" as the argument.
Thanks. I will fix this.
>
> > +{
> > + int sk_bpf_cb_flags;
> > +
> > + if (getopt)
> > + return -EINVAL;
> > +
> > + if (copy_from_sockptr(&sk_bpf_cb_flags, optval, sizeof(sk_bpf_cb_flags)))
>
> It is an unnecessary copy. Directly use the "int sk_bpf_cb_flags" arg instead.
>
> > + return -EFAULT;
>
> This should never happen.
>
> > +
> > + if (sk_bpf_cb_flags & ~SK_BPF_CB_MASK)
> > + return -EINVAL;
> > +
> > + sk->sk_bpf_cb_flags = sk_bpf_cb_flags;
> > +
> > + return 0;
> > +}
> > +
> > static int sol_socket_sockopt(struct sock *sk, int optname,
> > char *optval, int *optlen,
> > bool getopt)
> > @@ -5230,6 +5248,7 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> > case SO_MAX_PACING_RATE:
> > case SO_BINDTOIFINDEX:
> > case SO_TXREHASH:
> > + case SK_BPF_CB_FLAGS:
> > if (*optlen != sizeof(int))
> > return -EINVAL;
> > break;
> > @@ -5239,6 +5258,9 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> > return -EINVAL;
> > }
> >
> > + if (optname == SK_BPF_CB_FLAGS)
> > + return sk_bpf_set_cb_flags(sk, KERNEL_SOCKPTR(optval), getopt);
> > +
> > if (getopt) {
> > if (optname == SO_BINDTODEVICE)
> > return -EINVAL;
