On Fri, Apr 21, 2023 at 8:24 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 4/19/23 12:53 AM, Stanislav Fomichev wrote:
> > Over time, we've found out several special socket option cases which need
> > special treatment. And if BPF program doesn't handle them correctly, this
> > might EFAULT perfectly valid {g,s}setsockopt calls.
> >
> > The intention of the EFAULT was to make it apparent to the
> > developers that the program is doing something wrong.
> > However, this inadvertently might affect production workloads
> > with the BPF programs that are not too careful.
>
> Took in the first two for now. It would be good if the commit description
> in here could have more details for posterity given this is too vague.
Thanks! Will try to repost next week with more details.
> > Let's try to minimize the chance of BPF program screwing up userspace
> > by ignoring the output of those BPF programs (instead of returning
> > EFAULT to the userspace). pr_info_ratelimited those cases to
> > the dmesg to help with figuring out what's going wrong.
> >
> > Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> > Suggested-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
> > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> > ---
> > kernel/bpf/cgroup.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index a06e118a9be5..af4d20864fb4 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> > @@ -1826,7 +1826,9 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
> > ret = 1;
> > } else if (ctx.optlen > max_optlen || ctx.optlen < -1) {
> > /* optlen is out of bounds */
> > - ret = -EFAULT;
> > + pr_info_ratelimited(
> > + "bpf setsockopt returned unexpected optlen=%d (max_optlen=%d)\n",
> > + ctx.optlen, max_optlen);
>
> Does it help any regular user if this log message is seen? I kind of doubt it a bit,
> it might create more confusion if log gets spammed with it, imo.
Agreed, but we need some way to let the users know that their bpf
program is doing the wrong thing (if they set the optlen too high for
example).
Any other better alternatives to expose those events?
> > } else {
> > /* optlen within bounds, run kernel handler */
> > ret = 0;
> > @@ -1922,7 +1924,9 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
> > goto out;
> >
> > if (optval && (ctx.optlen > max_optlen || ctx.optlen < 0)) {
> > - ret = -EFAULT;
> > + pr_info_ratelimited(
> > + "bpf getsockopt returned unexpected optlen=%d (max_optlen=%d)\n",
> > + ctx.optlen, max_optlen);
> > goto out;
> > }
> >
> >
>
