Message-ID: <Z/UyZNiYUq9qrZds@gmail.com>
Date: Tue, 8 Apr 2025 07:27:48 -0700
From: Breno Leitao <leitao@...ian.org>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com,
	horms@...nel.org, kernel-team@...a.com, kuba@...nel.org,
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	mathieu.desnoyers@...icios.com, mhiramat@...nel.org,
	ncardwell@...gle.com, netdev@...r.kernel.org, pabeni@...hat.com,
	rostedt@...dmis.org, song@...nel.org, yonghong.song@...ux.dev
Subject: Re: [PATCH net-next v2 2/2] trace: tcp: Add tracepoint for
 tcp_sendmsg_locked()

Hello Kuniyuki,

On Mon, Apr 07, 2025 at 06:00:35PM -0700, Kuniyuki Iwashima wrote:
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index ea8de00f669d0..270ce2c8c2d54 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -1160,6 +1160,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> >  		if (skb)
> >  			copy = size_goal - skb->len;
> >  
> > +		trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
> 
> skb could be NULL, so I think raw_tp_null_args[] needs to be updated.
> 
> Maybe try attaching a bpf prog that dereferences skb unconditionally
> and see if the bpf verifier rejects it.

I've been trying to dereference skb (while 0), and bpf verifier rejects
it.

Here is the code I wrote to test:

	SEC("tracepoint/tcp/tcp_sendmsg_locked")
	int bpf_tcp_sendmsg_locked(struct tcp_sendmsg_locked_args *ctx)
	{
		bpf_printk("TCP: skb_addr %p skb_len %d msg_left %d size_goal %d",
			ctx->skb_addr, ctx->skb_len, ctx->msg_left, ctx->size_goal);
	
		return 0;
	}

And it matches the tracepoint, but, trying to dereference skb_addr fails
in all forms. I tried with a proper dereference, or, something as simple
as the following, and the program is not loaded.

	bpf_printk("deref %d\n", *(int *) ctx->skb_addr);

Here is all it returns.

	libbpf: prog 'bpf_tcp_sendmsg_locked': BPF program load failed: Permission denied
	libbpf: prog 'bpf_tcp_sendmsg_locked': -- BEGIN PROG LOAD LOG --
	0: R1=ctx() R10=fp0
	; int bpf_tcp_sendmsg_locked(struct tcp_sendmsg_locked_args *ctx) @ tcp_sendmsg_locked_bpf.c:16
	0: (bf) r6 = r1                       ; R1=ctx() R6_w=ctx()
	; bpf_printk("TCP: skb_addr %p skb_len %d msg_left %d size_goal %d", @ tcp_sendmsg_locked_bpf.c:19
	1: (79) r1 = *(u64 *)(r6 +8)          ; R1_w=scalar() R6_w=ctx()
	2: (7b) *(u64 *)(r10 -32) = r1        ; R1_w=scalar(id=1) R10=fp0 fp-32_w=scalar(id=1)
	3: (61) r1 = *(u32 *)(r6 +16)         ; R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) R6_w=ctx()
	4: (67) r1 <<= 32                     ; R1_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
	5: (c7) r1 s>>= 32                    ; R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
	6: (7b) *(u64 *)(r10 -24) = r1        ; R1_w=scalar(id=2,smin=0xffffffff80000000,smax=0x7fffffff) R10=fp0 fp-24_w=scalar(id=2,smin=0xffffffff80000000,smax=0x7fffffff)
	7: (61) r1 = *(u32 *)(r6 +20)         ; R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) R6_w=ctx()
	8: (67) r1 <<= 32                     ; R1_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
	9: (c7) r1 s>>= 32                    ; R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
	10: (7b) *(u64 *)(r10 -16) = r1       ; R1_w=scalar(id=3,smin=0xffffffff80000000,smax=0x7fffffff) R10=fp0 fp-16_w=scalar(id=3,smin=0xffffffff80000000,smax=0x7fffffff)
	11: (61) r1 = *(u32 *)(r6 +24)        ; R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff)) R6_w=ctx()
	12: (67) r1 <<= 32                    ; R1_w=scalar(smax=0x7fffffff00000000,umax=0xffffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
	13: (c7) r1 s>>= 32                   ; R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
	14: (7b) *(u64 *)(r10 -8) = r1        ; R1_w=scalar(id=4,smin=0xffffffff80000000,smax=0x7fffffff) R10=fp0 fp-8_w=scalar(id=4,smin=0xffffffff80000000,smax=0x7fffffff)
	15: (bf) r3 = r10                     ; R3_w=fp0 R10=fp0
	16: (07) r3 += -32                    ; R3_w=fp-32
	17: (18) r1 = 0xff1100010965cdd8      ; R1_w=map_value(map=tcp_send.rodata,ks=4,vs=63)
	19: (b7) r2 = 53                      ; R2_w=53
	20: (b7) r4 = 32                      ; R4_w=32
	21: (85) call bpf_trace_vprintk#177   ; R0_w=scalar()
	; bpf_printk("deref %d\n", *(int *) ctx->skb_addr); @ tcp_sendmsg_locked_bpf.c:22
	22: (79) r1 = *(u64 *)(r6 +8)         ; R1_w=scalar() R6_w=ctx()
	23: (61) r3 = *(u32 *)(r1 +0)
	R1 invalid mem access 'scalar'
	processed 23 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
	-- END PROG LOAD LOG --
	libbpf: prog 'bpf_tcp_sendmsg_locked': failed to load: -13
	libbpf: failed to load object 'tcp_sendmsg_locked_bpf.o'
	Failed to load BPF object: -13

I've pushed this example to the following URL, if you want to experiment
as well:

	https://github.com/leitao/debug/blob/main/bpf/tracepoint/tcp_sendmsg_locked_bpf.c

> See this commit for the similar issue:
> 
> commit 5da7e15fb5a12e78de974d8908f348e279922ce9
> Author: Kuniyuki Iwashima <kuniyu@...zon.com>
> Date:   Fri Jan 31 19:01:42 2025 -0800
> 
>     net: Add rx_skb of kfree_skb to raw_tp_null_args[].
> 

Thanks for the heads-up. I can populate raw_tp_null_args with this new
tracepoint function, if that is the right thing to do, even without
being able to reproduce the issue above.

Thanks for the review,
--breno

Powered by blists - more mailing lists