| 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: <87v81enawc.fsf@cloudflare.com>
Date: Tue, 09 Jul 2024 11:59:47 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Michal Luczaj <mhal@...x.co>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
john.fastabend@...il.com, kuniyu@...zon.com, Rao.Shoaib@...cle.com,
cong.wang@...edance.com
Subject: Re: [PATCH bpf v3 3/4] selftest/bpf: Parametrize AF_UNIX redir
functions to accept send() flags
On Sun, Jul 07, 2024 at 11:28 PM +02, Michal Luczaj wrote:
> Extend pairs_redir_to_connected() and unix_inet_redir_to_connected() with a
> send_flags parameter. Replace write() with send() allowing packets to be
> sent as MSG_OOB.
>
> Signed-off-by: Michal Luczaj <mhal@...x.co>
> ---
> .../selftests/bpf/prog_tests/sockmap_listen.c | 40 +++++++++++++------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index c075d376fcab..59e16f8f2090 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -1374,9 +1374,10 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map,
> }
> }
>
> -static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
> - int sock_mapfd, int nop_mapfd,
> - int verd_mapfd, enum redir_mode mode)
> +static void __pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
> + int sock_mapfd, int nop_mapfd,
> + int verd_mapfd, enum redir_mode mode,
> + int send_flags)
> {
> const char *log_prefix = redir_mode_str(mode);
> unsigned int pass;
> @@ -1396,11 +1397,9 @@ static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
> return;
> }
>
> - n = write(cli1, "a", 1);
> - if (n < 0)
> - FAIL_ERRNO("%s: write", log_prefix);
> + n = xsend(cli1, "a", 1, send_flags);
> if (n == 0)
> - FAIL("%s: incomplete write", log_prefix);
> + FAIL("%s: incomplete send", log_prefix);
> if (n < 1)
> return;
>
> @@ -1418,6 +1417,14 @@ static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
> FAIL("%s: incomplete recv", log_prefix);
> }
>
> +static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
> + int sock_mapfd, int nop_mapfd,
> + int verd_mapfd, enum redir_mode mode)
> +{
> + __pairs_redir_to_connected(cli0, peer0, cli1, peer1, sock_mapfd,
> + nop_mapfd, verd_mapfd, mode, 0);
> +}
> +
> static void unix_redir_to_connected(int sotype, int sock_mapfd,
> int verd_mapfd, enum redir_mode mode)
> {
> @@ -1815,10 +1822,9 @@ static void inet_unix_skb_redir_to_connected(struct test_sockmap_listen *skel,
> xbpf_prog_detach2(verdict, sock_map, BPF_SK_SKB_VERDICT);
> }
>
> -static void unix_inet_redir_to_connected(int family, int type,
> - int sock_mapfd, int nop_mapfd,
> - int verd_mapfd,
> - enum redir_mode mode)
> +static void __unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
> + int nop_mapfd, int verd_mapfd,
> + enum redir_mode mode, int send_flags)
> {
> int c0, c1, p0, p1;
> int sfd[2];
> @@ -1832,8 +1838,8 @@ static void unix_inet_redir_to_connected(int family, int type,
> goto close_cli0;
> c1 = sfd[0], p1 = sfd[1];
>
> - pairs_redir_to_connected(c0, p0, c1, p1,
> - sock_mapfd, nop_mapfd, verd_mapfd, mode);
> + __pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, nop_mapfd,
> + verd_mapfd, mode, send_flags);
>
> xclose(c1);
> xclose(p1);
> @@ -1842,6 +1848,14 @@ static void unix_inet_redir_to_connected(int family, int type,
> xclose(p0);
> }
>
> +static void unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
> + int nop_mapfd, int verd_mapfd,
> + enum redir_mode mode)
> +{
> + __unix_inet_redir_to_connected(family, type, sock_mapfd, nop_mapfd,
> + verd_mapfd, mode, 0);
> +}
> +
> static void unix_inet_skb_redir_to_connected(struct test_sockmap_listen *skel,
> struct bpf_map *inner_map, int family)
> {
I've got some cosmetic suggestions.
Instead of having two helper variants - with and without send_flags - we
could stick to just one and always pass send_flags. For readability I'd
use a constant for "no flags".
This way we keep the path open to convert
unix_inet_skb_redir_to_connected() to to a loop over a parameter
combination matrix, instead of open-coding multiple calls to
unix_inet_redir_to_connected() for each argument combination.
It seems doing it the current way, it is way too easy to miss
typos. Pretty sure we have another typo at [1], looks like should be
s/SOCK_DGRAM/SOCK_STREAM/.
[1]
https://elixir.bootlin.com/linux/v6.10-rc7/source/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c#L1863
See below for what I have in mind.
--8<--
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index 59e16f8f2090..3ddffaead2cd 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -29,6 +29,8 @@
#include "sockmap_helpers.h"
+#define NO_FLAGS 0
+
static void test_insert_invalid(struct test_sockmap_listen *skel __always_unused,
int family, int sotype, int mapfd)
{
@@ -1374,10 +1376,10 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map,
}
}
-static void __pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
- int sock_mapfd, int nop_mapfd,
- int verd_mapfd, enum redir_mode mode,
- int send_flags)
+static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
+ int sock_mapfd, int nop_mapfd,
+ int verd_mapfd, enum redir_mode mode,
+ int send_flags)
{
const char *log_prefix = redir_mode_str(mode);
unsigned int pass;
@@ -1417,14 +1419,6 @@ static void __pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
FAIL("%s: incomplete recv", log_prefix);
}
-static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,
- int sock_mapfd, int nop_mapfd,
- int verd_mapfd, enum redir_mode mode)
-{
- __pairs_redir_to_connected(cli0, peer0, cli1, peer1, sock_mapfd,
- nop_mapfd, verd_mapfd, mode, 0);
-}
-
static void unix_redir_to_connected(int sotype, int sock_mapfd,
int verd_mapfd, enum redir_mode mode)
{
@@ -1439,7 +1433,7 @@ static void unix_redir_to_connected(int sotype, int sock_mapfd,
goto close0;
c1 = sfd[0], p1 = sfd[1];
- pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode);
+ pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode, NO_FLAGS);
xclose(c1);
xclose(p1);
@@ -1729,7 +1723,7 @@ static void udp_redir_to_connected(int family, int sock_mapfd, int verd_mapfd,
if (err)
goto close_cli0;
- pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode);
+ pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode, NO_FLAGS);
xclose(c1);
xclose(p1);
@@ -1787,7 +1781,7 @@ static void inet_unix_redir_to_connected(int family, int type, int sock_mapfd,
if (err)
goto close;
- pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode);
+ pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, -1, verd_mapfd, mode, NO_FLAGS);
xclose(c1);
xclose(p1);
@@ -1822,9 +1816,9 @@ static void inet_unix_skb_redir_to_connected(struct test_sockmap_listen *skel,
xbpf_prog_detach2(verdict, sock_map, BPF_SK_SKB_VERDICT);
}
-static void __unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
- int nop_mapfd, int verd_mapfd,
- enum redir_mode mode, int send_flags)
+static void unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
+ int nop_mapfd, int verd_mapfd,
+ enum redir_mode mode, int send_flags)
{
int c0, c1, p0, p1;
int sfd[2];
@@ -1838,8 +1832,8 @@ static void __unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
goto close_cli0;
c1 = sfd[0], p1 = sfd[1];
- __pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, nop_mapfd,
- verd_mapfd, mode, send_flags);
+ pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, nop_mapfd,
+ verd_mapfd, mode, send_flags);
xclose(c1);
xclose(p1);
@@ -1848,14 +1842,6 @@ static void __unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
xclose(p0);
}
-static void unix_inet_redir_to_connected(int family, int type, int sock_mapfd,
- int nop_mapfd, int verd_mapfd,
- enum redir_mode mode)
-{
- __unix_inet_redir_to_connected(family, type, sock_mapfd, nop_mapfd,
- verd_mapfd, mode, 0);
-}
-
static void unix_inet_skb_redir_to_connected(struct test_sockmap_listen *skel,
struct bpf_map *inner_map, int family)
{
@@ -1872,31 +1858,31 @@ static void unix_inet_skb_redir_to_connected(struct test_sockmap_listen *skel,
skel->bss->test_ingress = false;
unix_inet_redir_to_connected(family, SOCK_DGRAM,
sock_map, -1, verdict_map,
- REDIR_EGRESS);
+ REDIR_EGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_DGRAM,
sock_map, -1, verdict_map,
- REDIR_EGRESS);
+ REDIR_EGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_DGRAM,
sock_map, nop_map, verdict_map,
- REDIR_EGRESS);
+ REDIR_EGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_STREAM,
sock_map, nop_map, verdict_map,
- REDIR_EGRESS);
+ REDIR_EGRESS, NO_FLAGS);
skel->bss->test_ingress = true;
unix_inet_redir_to_connected(family, SOCK_DGRAM,
sock_map, -1, verdict_map,
- REDIR_INGRESS);
+ REDIR_INGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_STREAM,
sock_map, -1, verdict_map,
- REDIR_INGRESS);
+ REDIR_INGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_DGRAM,
sock_map, nop_map, verdict_map,
- REDIR_INGRESS);
+ REDIR_INGRESS, NO_FLAGS);
unix_inet_redir_to_connected(family, SOCK_STREAM,
sock_map, nop_map, verdict_map,
- REDIR_INGRESS);
+ REDIR_INGRESS, NO_FLAGS);
xbpf_prog_detach2(verdict, sock_map, BPF_SK_SKB_VERDICT);
}
Powered by blists - more mailing lists