public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access
@ 2026-04-22 18:37 Suchit Karunakaran
  2026-04-22 21:21 ` Alexei Starovoitov
  2026-04-22 22:05 ` sashiko-bot
  0 siblings, 2 replies; 4+ messages in thread
From: Suchit Karunakaran @ 2026-04-22 18:37 UTC (permalink / raw)
  To: andrii, ast, daniel, eddyz87, memxor, shuah
  Cc: jolsa, martin.lau, song, yonghong.song, bpf, linux-kselftest,
	linux-kernel, Suchit Karunakaran

Use direct field access instead of bpf_probe_read_kernel() for
nlk->groups[0] and inode->i_ino, and clean up related code.

Removes obsolete FIXME comments and unused variable.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 .../testing/selftests/bpf/progs/bpf_iter_netlink.c  | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
index 00b2ceae81fb..5930bf8899d2 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
@@ -17,7 +17,6 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
 	struct seq_file *seq = ctx->meta->seq;
 	struct netlink_sock *nlk = ctx->sk;
 	unsigned long group, ino;
-	struct inode *inode;
 	struct socket *sk;
 	struct sock *s;
 
@@ -35,10 +34,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
 	if (!nlk->groups)  {
 		group = 0;
 	} else {
-		/* FIXME: temporary use bpf_probe_read_kernel here, needs
-		 * verifier support to do direct access.
-		 */
-		bpf_probe_read_kernel(&group, sizeof(group), &nlk->groups[0]);
+		group = nlk->groups[0];
 	}
 	BPF_SEQ_PRINTF(seq, "%-10u %08x %-8d %-8d %-5d %-8d ",
 		       nlk->portid, (u32)group,
@@ -50,12 +46,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
 	if (!sk) {
 		ino = 0;
 	} else {
-		/* FIXME: container_of inside SOCK_INODE has a forced
-		 * type conversion, and direct access cannot be used
-		 * with current verifier.
-		 */
-		inode = SOCK_INODE(sk);
-		bpf_probe_read_kernel(&ino, sizeof(ino), &inode->i_ino);
+		ino = SOCK_INODE(sk)->i_ino;
 	}
 	BPF_SEQ_PRINTF(seq, "%-8u %-8lu\n", s->sk_drops.counter, ino);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access
  2026-04-22 18:37 [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access Suchit Karunakaran
@ 2026-04-22 21:21 ` Alexei Starovoitov
  2026-04-23  6:56   ` Suchit Karunakaran
  2026-04-22 22:05 ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-04-22 21:21 UTC (permalink / raw)
  To: Suchit Karunakaran
  Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Eduard,
	Kumar Kartikeya Dwivedi, Shuah Khan, Jiri Olsa, Martin KaFai Lau,
	Song Liu, Yonghong Song, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	LKML

On Wed, Apr 22, 2026 at 11:38 AM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> Use direct field access instead of bpf_probe_read_kernel() for
> nlk->groups[0] and inode->i_ino, and clean up related code.
>
> Removes obsolete FIXME comments and unused variable.
>
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  .../testing/selftests/bpf/progs/bpf_iter_netlink.c  | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> index 00b2ceae81fb..5930bf8899d2 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> @@ -17,7 +17,6 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
>         struct seq_file *seq = ctx->meta->seq;
>         struct netlink_sock *nlk = ctx->sk;
>         unsigned long group, ino;
> -       struct inode *inode;
>         struct socket *sk;
>         struct sock *s;
>
> @@ -35,10 +34,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
>         if (!nlk->groups)  {
>                 group = 0;
>         } else {
> -               /* FIXME: temporary use bpf_probe_read_kernel here, needs
> -                * verifier support to do direct access.
> -                */
> -               bpf_probe_read_kernel(&group, sizeof(group), &nlk->groups[0]);

Please stop sending patches to remove TODO and FIXME.

pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access
  2026-04-22 18:37 [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access Suchit Karunakaran
  2026-04-22 21:21 ` Alexei Starovoitov
@ 2026-04-22 22:05 ` sashiko-bot
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-04-22 22:05 UTC (permalink / raw)
  To: Suchit Karunakaran; +Cc: bpf

Sashiko AI review found 2 potential issue(s):
- [Medium] BPF verifier will reject the program due to an out-of-bounds access on `SOCK_INODE(sk)->i_ino`.
- [Medium] BPF verifier will reject the program due to an invalid memory access when dereferencing `nlk->groups[0]`.
--

commit 9c46bc4644e11b2c3abf78e2ca55fa73028182f8
Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>

selftests/bpf: replace bpf_probe_read_kernel with direct access

Use direct field access instead of bpf_probe_read_kernel() for
nlk->groups[0] and inode->i_ino, and clean up related code.

> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> index 00b2ceae81fb0..5930bf8899d21 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> @@ -35,10 +34,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
>  	if (!nlk->groups)  {
>  		group = 0;
>  	} else {
> -		/* FIXME: temporary use bpf_probe_read_kernel here, needs
> -		 * verifier support to do direct access.
> -		 */
> -		bpf_probe_read_kernel(&group, sizeof(group), &nlk->groups[0]);
> +		group = nlk->groups[0];

Does the verifier know the size of the memory pointed to by nlk->groups?

The groups field in struct netlink_sock is a pointer to a scalar
(unsigned long *). When the BPF program reads this pointer from the struct,
the verifier tracks it as a generic pointer with a known memory size of 0,
because BTF does not encode the size of the dynamically allocated array.

Will the direct array access nlk->groups[0] be rejected by the verifier's
bounds check?

>  	}
>  	BPF_SEQ_PRINTF(seq, "%-10u %08x %-8d %-8d %-5d %-8d ",
>  		       nlk->portid, (u32)group,
> @@ -50,12 +46,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
>  	if (!sk) {
>  		ino = 0;
>  	} else {
> -		/* FIXME: container_of inside SOCK_INODE has a forced
> -		 * type conversion, and direct access cannot be used
> -		 * with current verifier.
> -		 */
> -		inode = SOCK_INODE(sk);
> -		bpf_probe_read_kernel(&ino, sizeof(ino), &inode->i_ino);
> +		ino = SOCK_INODE(sk)->i_ino;

Will the verifier reject this as an out-of-bounds access?

The SOCK_INODE macro relies on container_of to cast a struct socket pointer
to a struct socket_alloc. In BPF, standard C pointer arithmetic does not
implicitly update the BTF type tracked by the verifier.

The verifier will still track sk as a struct socket, but with an added
offset. Because vfs_inode sits after socket in struct socket_alloc,
accessing i_ino uses an offset that exceeds sizeof(struct socket).

Should this use bpf_core_cast(sk, struct socket_alloc) to update the
verifier's tracked BTF type instead?

>  	}
>  	BPF_SEQ_PRINTF(seq, "%-8u %-8lu\n", s->sk_drops.counter, ino);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422183757.15069-1-suchitkarunakaran@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access
  2026-04-22 21:21 ` Alexei Starovoitov
@ 2026-04-23  6:56   ` Suchit Karunakaran
  0 siblings, 0 replies; 4+ messages in thread
From: Suchit Karunakaran @ 2026-04-23  6:56 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Eduard,
	Kumar Kartikeya Dwivedi, Shuah Khan, Jiri Olsa, Martin KaFai Lau,
	Song Liu, Yonghong Song, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	LKML

On Thu, 23 Apr 2026 at 02:51, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Apr 22, 2026 at 11:38 AM Suchit Karunakaran
> <suchitkarunakaran@gmail.com> wrote:
> >
> > Use direct field access instead of bpf_probe_read_kernel() for
> > nlk->groups[0] and inode->i_ino, and clean up related code.
> >
> > Removes obsolete FIXME comments and unused variable.
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >  .../testing/selftests/bpf/progs/bpf_iter_netlink.c  | 13 ++-----------
> >  1 file changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> > index 00b2ceae81fb..5930bf8899d2 100644
> > --- a/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_netlink.c
> > @@ -17,7 +17,6 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
> >         struct seq_file *seq = ctx->meta->seq;
> >         struct netlink_sock *nlk = ctx->sk;
> >         unsigned long group, ino;
> > -       struct inode *inode;
> >         struct socket *sk;
> >         struct sock *s;
> >
> > @@ -35,10 +34,7 @@ int dump_netlink(struct bpf_iter__netlink *ctx)
> >         if (!nlk->groups)  {
> >                 group = 0;
> >         } else {
> > -               /* FIXME: temporary use bpf_probe_read_kernel here, needs
> > -                * verifier support to do direct access.
> > -                */
> > -               bpf_probe_read_kernel(&group, sizeof(group), &nlk->groups[0]);
>
> Please stop sending patches to remove TODO and FIXME.
>
> pw-bot: cr

Sorry about that. I'll test it more rigorously next time and ensure
the patch is sensible.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-23  6:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-22 18:37 [PATCH] selftests/bpf: replace bpf_probe_read_kernel with direct access Suchit Karunakaran
2026-04-22 21:21 ` Alexei Starovoitov
2026-04-23  6:56   ` Suchit Karunakaran
2026-04-22 22:05 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox