Skip to content

Commit b99f27e

Browse files
ameryhungMartin KaFai Lau
authored andcommitted
selftests/bpf: Fix stdout race condition in traffic monitor
Fix a race condition between the main test_progs thread and the traffic monitoring thread. The traffic monitor thread tries to print a line using multiple printf and use flockfile() to prevent the line from being torn apart. Meanwhile, the main thread doing io redirection can reassign or close stdout when going through tests. A deadlock as shown below can happen. main traffic_monitor_thread ==== ====================== show_transport() -> flockfile(stdout) stdio_hijack_init() -> stdout = open_memstream(log_buf, log_cnt); ... env.subtest_state->stdout_saved = stdout; ... funlockfile(stdout) stdio_restore_cleanup() -> fclose(env.subtest_state->stdout_saved); After the traffic monitor thread lock stdout, A new memstream can be assigned to stdout by the main thread. Therefore, the traffic monitor thread later will not be able to unlock the original stdout. As the main thread tries to access the old stdout, it will hang indefinitely as it is still locked by the traffic monitor thread. The deadlock can be reproduced by running test_progs repeatedly with traffic monitor enabled: for ((i=1;i<=100;i++)); do ./test_progs -a flow_dissector_skb* -m '*' done Fix this by only calling printf once and remove flockfile()/funlockfile(). Signed-off-by: Amery Hung <ameryhung@gmail.com> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20250213233217.553258-1-ameryhung@gmail.com
1 parent c83e2d9 commit b99f27e

File tree

1 file changed

+13
-20
lines changed

1 file changed

+13
-20
lines changed

tools/testing/selftests/bpf/network_helpers.c

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -788,12 +788,13 @@ static const char *pkt_type_str(u16 pkt_type)
788788
return "Unknown";
789789
}
790790

791+
#define MAX_FLAGS_STRLEN 21
791792
/* Show the information of the transport layer in the packet */
792793
static void show_transport(const u_char *packet, u16 len, u32 ifindex,
793794
const char *src_addr, const char *dst_addr,
794795
u16 proto, bool ipv6, u8 pkt_type)
795796
{
796-
char *ifname, _ifname[IF_NAMESIZE];
797+
char *ifname, _ifname[IF_NAMESIZE], flags[MAX_FLAGS_STRLEN] = "";
797798
const char *transport_str;
798799
u16 src_port, dst_port;
799800
struct udphdr *udp;
@@ -834,29 +835,21 @@ static void show_transport(const u_char *packet, u16 len, u32 ifindex,
834835

835836
/* TCP or UDP*/
836837

837-
flockfile(stdout);
838+
if (proto == IPPROTO_TCP)
839+
snprintf(flags, MAX_FLAGS_STRLEN, "%s%s%s%s",
840+
tcp->fin ? ", FIN" : "",
841+
tcp->syn ? ", SYN" : "",
842+
tcp->rst ? ", RST" : "",
843+
tcp->ack ? ", ACK" : "");
844+
838845
if (ipv6)
839-
printf("%-7s %-3s IPv6 %s.%d > %s.%d: %s, length %d",
846+
printf("%-7s %-3s IPv6 %s.%d > %s.%d: %s, length %d%s\n",
840847
ifname, pkt_type_str(pkt_type), src_addr, src_port,
841-
dst_addr, dst_port, transport_str, len);
848+
dst_addr, dst_port, transport_str, len, flags);
842849
else
843-
printf("%-7s %-3s IPv4 %s:%d > %s:%d: %s, length %d",
850+
printf("%-7s %-3s IPv4 %s:%d > %s:%d: %s, length %d%s\n",
844851
ifname, pkt_type_str(pkt_type), src_addr, src_port,
845-
dst_addr, dst_port, transport_str, len);
846-
847-
if (proto == IPPROTO_TCP) {
848-
if (tcp->fin)
849-
printf(", FIN");
850-
if (tcp->syn)
851-
printf(", SYN");
852-
if (tcp->rst)
853-
printf(", RST");
854-
if (tcp->ack)
855-
printf(", ACK");
856-
}
857-
858-
printf("\n");
859-
funlockfile(stdout);
852+
dst_addr, dst_port, transport_str, len, flags);
860853
}
861854

862855
static void show_ipv6_packet(const u_char *packet, u32 ifindex, u8 pkt_type)

0 commit comments

Comments
 (0)