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.
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.
} 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;
}
