Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen #5011

Closed
wants to merge 4 commits into from
Closed

bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen #5011

wants to merge 4 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a464411
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6ec7be9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: bf06c93
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f1f5553
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 31f4f81
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 31f4f81
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=743665
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: c39028b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=744231
version: 3

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot added V3 and removed V2 labels May 1, 2023
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 577c34b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=744231
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 577c34b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=744231
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: fedf992
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=745069
version: 4

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 7866fc6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=745069
version: 4

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: fbc0b02
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=745069
version: 4

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6953518
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=745069
version: 4

@fomichev
bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen
d4ba08a 
With the way the hooks implemented right now, we have a special
condition: optval larger than PAGE_SIZE will expose only first 4k into
BPF; any modifications to the optval are ignored. If the BPF program
doesn't handle this condition by resetting optlen to 0,
the userspace will get EFAULT.

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 (i.e., returning EFAULT
for perfectly valid setsockopt/getsockopt calls).

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_once those cases to
the dmesg to help with figuring out what's going wrong.

Fixes: 0d01da6 ("bpf: implement getsockopt and setsockopt hooks")
Suggested-by: Martin KaFai Lau <martin.lau@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
@fomichev
selftests/bpf: Update EFAULT {g,s}etsockopt selftests
c82e28a 
Instead of assuming EFAULT, let's assume the BPF program's
output is ignored.

Remove "getsockopt: deny arbitrary ctx->retval" because it
was actually testing optlen. We have separate set of tests
for retval.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@fomichev
selftests/bpf: Correctly handle optlen > 4096
7235b8d 
Even though it's not relevant in selftests, the people
might still copy-paste from them. So let's take care
of optlen > 4096 cases explicitly.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f4dea96
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=745069
version: 4

@fomichev
bpf: Document EFAULT changes for sockopt
da8092f 
And add examples for how to correctly handle large optlens.
This is less relevant now when we don't EFAULT anymore, but
that's still the correct thing to do.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=745069 expired. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/743665=>bpf-next branch May 8, 2023 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next changes-requested V4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@fomichev