From: Martin KaFai Lau <martin.lau@linux.dev>
To: Stanislav Fomichev <sdf@google.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org,
Martin KaFai Lau <martin.lau@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH bpf-next 3/6] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen
Date: Wed, 26 Apr 2023 11:07:13 -0700 [thread overview]
Message-ID: <9864acbe-7118-d7b5-0287-7737f3135c30@linux.dev> (raw)
In-Reply-To: <CAKH8qBtuz0DYrsdgoX2_McOYFSES2_z9+BWcj+XczQZ_Fr6_KQ@mail.gmail.com>
On 4/26/23 10:27 AM, Stanislav Fomichev wrote:
>>> As per above, I'll stick a line to the dmest (similar
>>> bpf_warn_invalid_xdp_action), at least to record that this has
>>> happened once.
>>> LMK if you or Danial still don't see a value in printing this..
>>
>> pr_info_once? hmm... I think it is ok-ish. At least not a warning.
>>
>> I think almost all of the time the bpf prog forgets to set it to 0 for the long
>> optval that it has no interest in. However, to suppress this pr_info_once,
>> setting optlen to 0 will disable the following cgroup-bpf prog from using the
>> optval as read-only. The case that the printk that may flag is that the bpf prog
>> did indeed want to change the long optval?
>
> The case we want to printk is where the prog changes some byte in the
> first 4k range of the optval and does not touch optlen (or maybe
> adjusts optlen to be >PAGE_SIZE and <original_optlen).
> I agree that it feels super corner-casy; but it feels like without
> some kind of hint, it would be impossible to figure out why it doesn't
> work. Or am I overblowing it?
I don't have a better idea how to flag this 'changing the first few bytes of a
long optval is not supported' either. I guess pr_info_once is ok-ish for now to
stop the bleeding in the most common case.
If it can separate the original_optlen > PAGE_SIZE case (ignore and no -EFAULT),
the message probably needs to be less alarming. "bpf setsockopt returned
unexpected optlen" may cause confusion when the bpf prog did not touch the
optval and optlen.
Hopefully this pr_info_once will disappear when the cgroup-bpf prog can directly
read/write the optval without pre-allocation.
next prev parent reply other threads:[~2023-04-26 18:07 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-18 22:53 [PATCH bpf-next 0/6] bpf: handle another corner case in getsockopt Stanislav Fomichev
2023-04-18 22:53 ` [PATCH bpf-next 1/6] bpf: Don't EFAULT for getsockopt with optval=NULL Stanislav Fomichev
2023-04-18 22:53 ` [PATCH bpf-next 2/6] selftests/bpf: Verify optval=NULL case Stanislav Fomichev
2023-04-18 22:53 ` [PATCH bpf-next 3/6] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen Stanislav Fomichev
2023-04-21 15:24 ` Daniel Borkmann
2023-04-21 16:09 ` Stanislav Fomichev
2023-04-25 0:56 ` Martin KaFai Lau
2023-04-25 17:12 ` Stanislav Fomichev
2023-04-25 18:31 ` Martin KaFai Lau
2023-04-26 17:27 ` Stanislav Fomichev
2023-04-26 18:07 ` Martin KaFai Lau [this message]
2023-04-18 22:53 ` [PATCH bpf-next 4/6] selftests/bpf: Update EFAULT {g,s}etsockopt selftests Stanislav Fomichev
2023-04-18 22:53 ` [PATCH bpf-next 5/6] selftests/bpf: Correctly handle optlen > 4096 Stanislav Fomichev
2023-04-18 22:53 ` [PATCH bpf-next 6/6] bpf: Document EFAULT changes for sockopt Stanislav Fomichev
2023-04-19 20:08 ` kernel test robot
2023-04-20 18:17 ` Stanislav Fomichev
2023-04-21 15:20 ` [PATCH bpf-next 0/6] bpf: handle another corner case in getsockopt patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9864acbe-7118-d7b5-0287-7737f3135c30@linux.dev \
--to=martin.lau@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@kernel.org \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.