| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAVpQUCowciRU-ES3tzr-m=UXZviLAhd-JC-5NLOOaO5kX6+8A@mail.gmail.com>
Date: Tue, 14 Oct 2025 16:23:26 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>,
Stanislav Fomichev <sdf@...ichev.me>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Neal Cardwell <ncardwell@...gle.com>, Willem de Bruijn <willemb@...gle.com>,
Mina Almasry <almasrymina@...gle.com>, Roman Gushchin <roman.gushchin@...ux.dev>,
Kuniyuki Iwashima <kuni1840@...il.com>, bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next/net 6/6] selftest: bpf: Add test for sk->sk_bypass_prot_mem.
On Tue, Oct 14, 2025 at 4:09 PM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
> On 10/6/25 5:07 PM, Kuniyuki Iwashima wrote:
> > +static int tcp_create_sockets(struct test_case *test_case, int sk[], int len)
> > +{
> > + int server, i;
> > +
> > + server = start_server(test_case->family, test_case->type, NULL, 0, 0);
> > + ASSERT_GE(server, 0, "start_server_str");
> > +
> > + /* Keep for-loop so we can change NR_SOCKETS easily. */
> > + for (i = 0; i < len; i += 2) {
> > + sk[i] = connect_to_fd(server, 0);
> > + if (sk[i] < 0) {
> > + ASSERT_GE(sk[i], 0, "connect_to_fd");
> > + return sk[i];
>
> The "server" fd is leaked, and...
>
> > + }
> > +
> > + sk[i + 1] = accept(server, NULL, NULL);
> > + if (sk[i + 1] < 0) {
> > + ASSERT_GE(sk[i + 1], 0, "accept");
> > + return sk[i + 1];
>
> same here.
Will fix them by err = sk[x] and break;.
>
> > + }
> > + }
> > +
> > + close(server);
> > +
> > + return 0;
> > +}
> > +
> > +static int udp_create_sockets(struct test_case *test_case, int sk[], int len)
> > +{
> > + int i, j, err, rcvbuf = BUF_TOTAL;
> > +
> > + /* Keep for-loop so we can change NR_SOCKETS easily. */
> > + for (i = 0; i < len; i += 2) {
> > + sk[i] = start_server(test_case->family, test_case->type, NULL, 0, 0);
> > + if (sk[i] < 0) {
> > + ASSERT_GE(sk[i], 0, "start_server");
> > + return sk[i];
> > + }
> > +
> > + sk[i + 1] = connect_to_fd(sk[i], 0);
> > + if (sk[i + 1] < 0) {
> > + ASSERT_GE(sk[i + 1], 0, "connect_to_fd");
> > + return sk[i + 1];
> > + }
> > +
> > + err = connect_fd_to_fd(sk[i], sk[i + 1], 0);
> > + if (err) {
> > + ASSERT_EQ(err, 0, "connect_fd_to_fd");
> > + return err;
> > + }
> > +
> > + for (j = 0; j < 2; j++) {
> > + err = setsockopt(sk[i + j], SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(int));
> > + if (err) {
> > + ASSERT_EQ(err, 0, "setsockopt(SO_RCVBUF)");
> > + return err;
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
>
>
> > +
> > +static int check_bypass(struct test_case *test_case,
> > + struct sk_bypass_prot_mem *skel, bool bypass)
> > +{
> > + char buf[BUF_SINGLE] = {};
> > + long memory_allocated[2];
> > + int sk[NR_SOCKETS] = {};
> > + int err, i, j;
> > +
> > + err = test_case->create_sockets(test_case, sk, ARRAY_SIZE(sk));
> > + if (err)
> > + goto close;
> > +
> > + memory_allocated[0] = test_case->get_memory_allocated(test_case, skel);
> > +
> > + /* allocate pages >= NR_PAGES */
> > + for (i = 0; i < ARRAY_SIZE(sk); i++) {
> > + for (j = 0; j < NR_SEND; j++) {
> > + int bytes = send(sk[i], buf, sizeof(buf), 0);
> > +
> > + /* Avoid too noisy logs when something failed. */
> > + if (bytes != sizeof(buf)) {
> > + ASSERT_EQ(bytes, sizeof(buf), "send");
> > + if (bytes < 0) {
> > + err = bytes;
> > + goto drain;
> > + }
> > + }
> > + }
> > + }
> > +
> > + memory_allocated[1] = test_case->get_memory_allocated(test_case, skel);
> > +
> > + if (bypass)
> > + ASSERT_LE(memory_allocated[1], memory_allocated[0] + 10, "bypass");
> > + else
> > + ASSERT_GT(memory_allocated[1], memory_allocated[0] + NR_PAGES, "no bypass");
> > +
> > +drain:
> > + if (test_case->type == SOCK_DGRAM) {
> > + /* UDP starts purging sk->sk_receive_queue after one RCU
> > + * grace period, then udp_memory_allocated goes down,
> > + * so drain the queue before close().
> > + */
> > + for (i = 0; i < ARRAY_SIZE(sk); i++) {
> > + for (j = 0; j < NR_SEND; j++) {
> > + int bytes = recv(sk[i], buf, 1, MSG_DONTWAIT | MSG_TRUNC);
> > +
> > + if (bytes == sizeof(buf))
> > + continue;
> > + if (bytes != -1 || errno != EAGAIN)
> > + PRINT_FAIL("bytes: %d, errno: %s\n", bytes, strerror(errno));
> > + break;
> > + }
> > + }
> > + }
> > +
> > +close:
> > + for (i = 0; i < ARRAY_SIZE(sk); i++)
> > + close(sk[i]);
>
> It could close(0) here depending on how the "->create_sockets()" above has
> failed. The fd 0 could be something useful for the test_progs.
Will add if (sk[i]) guard here.
>
> Other than that, the set lgtm. Please re-spin and carry the review/ack tags.
Thank you, Martin!
>
> pw-bot: cr
>
> > +
> > + return err;
> > +}
> > +
Powered by blists - more mailing lists