Stanislav Fomichev <sdf@xxxxxxxxxx> [Thu, 2020-04-30 16:32 -0700]:
> Currently, bpf_getsockopt and bpf_setsockopt helpers operate on the
> 'struct bpf_sock_ops' context in BPF_PROG_TYPE_SOCK_OPS program.
> Let's generalize them and make them available for 'struct bpf_sock_addr'.
> That way, in the future, we can allow those helpers in more places.
>
> As an example, let's expose those 'struct bpf_sock_addr' based helpers to
> BPF_CGROUP_INET{4,6}_CONNECT hooks. That way we can override CC before the
> connection is made.
>
> v3:
> * Expose custom helpers for bpf_sock_addr context instead of doing
> generic bpf_sock argument (as suggested by Daniel). Even with
> try_socket_lock that doesn't sleep we have a problem where context sk
> is already locked and socket lock is non-nestable.
>
> v2:
> * s/BPF_PROG_TYPE_CGROUP_SOCKOPT/BPF_PROG_TYPE_SOCK_OPS/
>
> Cc: John Fastabend <john.fastabend@xxxxxxxxx>
> Cc: Martin KaFai Lau <kafai@xxxxxx>
> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
...
> SEC("cgroup/connect4")
> int connect_v4_prog(struct bpf_sock_addr *ctx)
> {
> @@ -66,6 +108,10 @@ int connect_v4_prog(struct bpf_sock_addr *ctx)
>
> bpf_sk_release(sk);
>
> + /* Rewrite congestion control. */
> + if (ctx->type == SOCK_STREAM && set_cc(ctx))
> + return 0;
Hi Stas,
This new check breaks one of tests in test_sock_addr:
root@arch-fb-vm1:/home/rdna/bpf-next/tools/testing/selftests/bpf ./test_sock_addr.sh
...
(test_sock_addr.c:1199: errno: Operation not permitted) Fail to connect to server
Test case: connect4: rewrite IP & TCP port .. [FAIL]
...
Summary: 34 PASSED, 1 FAILED
What the test does is it sets up TCPv4 server:
[pid 386] socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 6
[pid 386] bind(6, {sa_family=AF_INET, sin_port=htons(4444), sin_addr=inet_addr("127.0.0.1")}, 128) = 0
[pid 386] listen(6, 128) = 0
Then tries to connect to a fake IPv4:port and this connect4 program
should redirect it to that TCP server, but only if every field in
context has expected value.
But after that commit program started denying the connect:
[pid 386] socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 7
[pid 386] connect(7, {sa_family=AF_INET, sin_port=htons(4040), sin_addr=inet_addr("192.168.1.254")}, 128) = -1 EPERM (Operation not permitted)
(test_sock_addr.c:1201: errno: Operation not permitted) Fail to connect to server
Test case: connect4: rewrite IP & TCP port .. [FAIL]
I verified that commenting out this new `if` fixes the problem, but
haven't spent time root-causing it. Could you please look at it?
Thanks.
--
Andrey Ignatov
