Skip to content

Commit

Permalink
selftests/bpf: Fix stdout race condition in traffic monitor
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Amery Hung authored and Martin KaFai Lau committed Feb 14, 2025
1 parent c83e2d9 commit b99f27e
Showing 1 changed file with 13 additions and 20 deletions.
33 changes: 13 additions & 20 deletions tools/testing/selftests/bpf/network_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -788,12 +788,13 @@ static const char *pkt_type_str(u16 pkt_type)
return "Unknown";
}

#define MAX_FLAGS_STRLEN 21
/* Show the information of the transport layer in the packet */
static void show_transport(const u_char *packet, u16 len, u32 ifindex,
const char *src_addr, const char *dst_addr,
u16 proto, bool ipv6, u8 pkt_type)
{
char *ifname, _ifname[IF_NAMESIZE];
char *ifname, _ifname[IF_NAMESIZE], flags[MAX_FLAGS_STRLEN] = "";
const char *transport_str;
u16 src_port, dst_port;
struct udphdr *udp;
Expand Down Expand Up @@ -834,29 +835,21 @@ static void show_transport(const u_char *packet, u16 len, u32 ifindex,

/* TCP or UDP*/

flockfile(stdout);
if (proto == IPPROTO_TCP)
snprintf(flags, MAX_FLAGS_STRLEN, "%s%s%s%s",
tcp->fin ? ", FIN" : "",
tcp->syn ? ", SYN" : "",
tcp->rst ? ", RST" : "",
tcp->ack ? ", ACK" : "");

if (ipv6)
printf("%-7s %-3s IPv6 %s.%d > %s.%d: %s, length %d",
printf("%-7s %-3s IPv6 %s.%d > %s.%d: %s, length %d%s\n",
ifname, pkt_type_str(pkt_type), src_addr, src_port,
dst_addr, dst_port, transport_str, len);
dst_addr, dst_port, transport_str, len, flags);
else
printf("%-7s %-3s IPv4 %s:%d > %s:%d: %s, length %d",
printf("%-7s %-3s IPv4 %s:%d > %s:%d: %s, length %d%s\n",
ifname, pkt_type_str(pkt_type), src_addr, src_port,
dst_addr, dst_port, transport_str, len);

if (proto == IPPROTO_TCP) {
if (tcp->fin)
printf(", FIN");
if (tcp->syn)
printf(", SYN");
if (tcp->rst)
printf(", RST");
if (tcp->ack)
printf(", ACK");
}

printf("\n");
funlockfile(stdout);
dst_addr, dst_port, transport_str, len, flags);
}

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

0 comments on commit b99f27e

Please sign in to comment.