netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: davem@davemloft.net, dsahern@kernel.org, edumazet@google.com,
	horms@kernel.org, kernel-team@meta.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	mathieu.desnoyers@efficios.com, mhiramat@kernel.org,
	ncardwell@google.com, netdev@vger.kernel.org, pabeni@redhat.com,
	rostedt@goodmis.org, song@kernel.org, yonghong.song@linux.dev
Subject: Re: [PATCH net-next v2 2/2] trace: tcp: Add tracepoint for tcp_sendmsg_locked()
Date: Tue, 8 Apr 2025 07:27:48 -0700	[thread overview]
Message-ID: <Z/UyZNiYUq9qrZds@gmail.com> (raw)
In-Reply-To: <20250408010143.11193-1-kuniyu@amazon.com>

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@amazon.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

  reply	other threads:[~2025-04-08 14:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-07 13:40 [PATCH net-next v2 0/2] trace: add tracepoint to tcp_sendmsg_locked Breno Leitao
2025-04-07 13:40 ` [PATCH net-next v2 1/2] net: pass const to msg_data_left() Breno Leitao
2025-04-08  0:53   ` Kuniyuki Iwashima
2025-04-08 14:20   ` Eric Dumazet
2025-04-07 13:40 ` [PATCH net-next v2 2/2] trace: tcp: Add tracepoint for tcp_sendmsg_locked() Breno Leitao
2025-04-08  1:00   ` Kuniyuki Iwashima
2025-04-08 14:27     ` Breno Leitao [this message]
2025-04-08 15:16       ` David Ahern
2025-04-08 17:01         ` Breno Leitao
2025-04-08 17:12           ` Kuniyuki Iwashima
2025-04-08 18:06             ` Breno Leitao
2025-04-08  1:05   ` Kuniyuki Iwashima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z/UyZNiYUq9qrZds@gmail.com \
    --to=leitao@debian.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@amazon.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).