BPF List
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@google.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org,
	Martin KaFai Lau <martin.lau@kernel.org>
Subject: [PATCH bpf-next 3/6] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen
Date: Tue, 18 Apr 2023 15:53:40 -0700	[thread overview]
Message-ID: <20230418225343.553806-4-sdf@google.com> (raw)
In-Reply-To: <20230418225343.553806-1-sdf@google.com>

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.

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@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 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);
 	} 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;
 	}
 
-- 
2.40.0.634.g4ca3ef3211-goog


  parent reply	other threads:[~2023-04-18 22:53 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 ` Stanislav Fomichev [this message]
2023-04-21 15:24   ` [PATCH bpf-next 3/6] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen 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
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=20230418225343.553806-4-sdf@google.com \
    --to=sdf@google.com \
    --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=martin.lau@linux.dev \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox