[PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[Paul Chaignon]

Syzkaller found a kernel warning on the following sock_addr program:

    0: r0 = 0
    1: r2 = *(u32 *)(r1 +60)
    2: exit

which triggers:

    verifier bug: error during ctx access conversion (0)

This is happening because offset 60 in bpf_sock_addr corresponds to an
implicit padding of 4 bytes, right after msg_src_ip4. Access to this
padding isn't rejected in sock_addr_is_valid_access and it thus later
fails to convert the access.

This patch fixes it by explicitly checking the various fields of
bpf_sock_addr in sock_addr_is_valid_access.

I checked the other ctx structures and is_valid_access functions and
didn't find any other similar cases. Other cases of (properly handled)
padding are covered in new tests in a subsequent patch.

Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
Reported-by: syzbot+136ca59d411f92e821b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=136ca59d411f92e821b7
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 net/core/filter.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index da391e2b0788..9ac58960e59e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -9284,13 +9284,19 @@ static bool sock_addr_is_valid_access(int off, int size,
 			return false;
 		info->reg_type = PTR_TO_SOCKET;
 		break;
-	default:
+	case bpf_ctx_range(struct bpf_sock_addr, user_family):
+	case bpf_ctx_range(struct bpf_sock_addr, family):
+	case bpf_ctx_range(struct bpf_sock_addr, type):
+	case bpf_ctx_range(struct bpf_sock_addr, protocol):
 		if (type == BPF_READ) {
 			if (size != size_default)
 				return false;
 		} else {
 			return false;
 		}
+		break;
+	default:
+		return false;
 	}
 
 	return true;
-- 
2.43.0

[PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h

[Paul Chaignon]

Move the sizeof_field and offsetofend macros from individual test files
to the common bpf_misc.h to avoid duplication.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 tools/testing/selftests/bpf/progs/bpf_misc.h             | 4 ++++
 tools/testing/selftests/bpf/progs/test_cls_redirect.c    | 4 +---
 tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c | 5 +----
 tools/testing/selftests/bpf/progs/verifier_ctx.c         | 2 --
 tools/testing/selftests/bpf/progs/verifier_sock.c        | 4 ----
 5 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
index c1cfd297aabf..749bf235dc07 100644
--- a/tools/testing/selftests/bpf/progs/bpf_misc.h
+++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
@@ -156,6 +156,10 @@
 #define __imm_ptr(name) [name]"r"(&name)
 #define __imm_insn(name, expr) [name]"i"(*(long *)&(expr))
 
+#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+#define offsetofend(TYPE, MEMBER) \
+	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
+
 /* Magic constants used with __retval() */
 #define POINTER_VALUE	0xbadcafe
 #define TEST_DATA_LEN	64
diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
index f344c6835e84..fa1848650d4b 100644
--- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c
+++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
@@ -22,6 +22,7 @@
 
 #include "bpf_compiler.h"
 #include "test_cls_redirect.h"
+#include "bpf_misc.h"
 
 #pragma GCC diagnostic ignored "-Waddress-of-packed-member"
 
@@ -31,9 +32,6 @@
 #define INLINING __always_inline
 #endif
 
-#define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER) + sizeof((((TYPE *)0)->MEMBER)))
-
 #define IP_OFFSET_MASK (0x1FFF)
 #define IP_MF (0x2000)
 
diff --git a/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c b/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
index 5f4e87ee949a..1ecdf4c54de4 100644
--- a/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
+++ b/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
@@ -14,10 +14,7 @@
 #include <bpf/bpf_endian.h>
 #define BPF_PROG_TEST_TCP_HDR_OPTIONS
 #include "test_tcp_hdr_options.h"
-
-#ifndef sizeof_field
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-#endif
+#include "bpf_misc.h"
 
 __u8 test_kind = TCPOPT_EXP;
 __u16 test_magic = 0xeB9F;
diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c
index 424463094760..b927906aa305 100644
--- a/tools/testing/selftests/bpf/progs/verifier_ctx.c
+++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c
@@ -5,8 +5,6 @@
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-
 SEC("tc")
 __description("context stores via BPF_ATOMIC")
 __failure __msg("BPF_ATOMIC stores into R1 ctx is not allowed")
diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
index 0d5e56dffabb..bf88c644eb30 100644
--- a/tools/testing/selftests/bpf/progs/verifier_sock.c
+++ b/tools/testing/selftests/bpf/progs/verifier_sock.c
@@ -5,10 +5,6 @@
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-#define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
-
 struct {
 	__uint(type, BPF_MAP_TYPE_REUSEPORT_SOCKARRAY);
 	__uint(max_entries, 1);
-- 
2.43.0

Re: [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h

[Eduard Zingerman]

On Tue, 2025-09-16 at 17:18 +0200, Paul Chaignon wrote:
> Move the sizeof_field and offsetofend macros from individual test files
> to the common bpf_misc.h to avoid duplication.
> 
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

>  tools/testing/selftests/bpf/progs/bpf_misc.h             | 4 ++++
>  tools/testing/selftests/bpf/progs/test_cls_redirect.c    | 4 +---
>  tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c | 5 +----
>  tools/testing/selftests/bpf/progs/verifier_ctx.c         | 2 --
>  tools/testing/selftests/bpf/progs/verifier_sock.c        | 4 ----
>  5 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
> index c1cfd297aabf..749bf235dc07 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_misc.h
> +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
> @@ -156,6 +156,10 @@
>  #define __imm_ptr(name) [name]"r"(&name)
>  #define __imm_insn(name, expr) [name]"i"(*(long *)&(expr))
>  
> +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
> +#define offsetofend(TYPE, MEMBER) \
> +	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
                               ^
			Nit: this is a <tab>, not a <space> :)
> +
>  /* Magic constants used with __retval() */
>  #define POINTER_VALUE	0xbadcafe
>  #define TEST_DATA_LEN	64

[...]

[PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding

[Paul Chaignon]

This patch adds tests covering the various paddings in ctx structures.
In case of sk_lookup BPF programs, the behavior is a bit different
because accesses to the padding are explicitly allowed. Other cases
result in a clear reject from the verifier.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 .../selftests/bpf/progs/verifier_ctx.c        | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c
index b927906aa305..93d4b4d14dca 100644
--- a/tools/testing/selftests/bpf/progs/verifier_ctx.c
+++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c
@@ -262,4 +262,28 @@ narrow_load("sockops", bpf_sock_ops, skb_hwtstamp);
 unaligned_access("flow_dissector", __sk_buff, data);
 unaligned_access("netfilter", bpf_nf_ctx, skb);
 
+#define padding_access(type, ctx, prev_field, sz)			\
+	SEC(type)							\
+	__description("access on " #ctx " padding after " #prev_field)	\
+	__naked void padding_ctx_access_##ctx(void)			\
+	{								\
+		asm volatile ("						\
+		r1 = *(u%[size] *)(r1 + %[off]);			\
+		r0 = 0;							\
+		exit;"							\
+		:							\
+		: __imm_const(size, sz * 8),				\
+		  __imm_const(off, offsetofend(struct ctx, prev_field))	\
+		: __clobber_all);					\
+	}
+
+__failure __msg("invalid bpf_context access")
+padding_access("cgroup/bind4", bpf_sock_addr, msg_src_ip6[3], 4);
+
+__success
+padding_access("sk_lookup", bpf_sk_lookup, remote_port, 2);
+
+__failure __msg("invalid bpf_context access")
+padding_access("tc", __sk_buff, tstamp_type, 2);
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding

[Eduard Zingerman]

On Tue, 2025-09-16 at 17:19 +0200, Paul Chaignon wrote:
> This patch adds tests covering the various paddings in ctx structures.
> In case of sk_lookup BPF programs, the behavior is a bit different
> because accesses to the padding are explicitly allowed. Other cases
> result in a clear reject from the verifier.
> 
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> +#define padding_access(type, ctx, prev_field, sz)			\
> +	SEC(type)							\
> +	__description("access on " #ctx " padding after " #prev_field)	\
> +	__naked void padding_ctx_access_##ctx(void)			\
> +	{								\
> +		asm volatile ("						\
> +		r1 = *(u%[size] *)(r1 + %[off]);			\
> +		r0 = 0;							\
> +		exit;"							\
> +		:							\
> +		: __imm_const(size, sz * 8),				\
                                    ^^^^^^
			Surprised this works, but nice.

> +		  __imm_const(off, offsetofend(struct ctx, prev_field))	\
> +		: __clobber_all);					\
> +	}

[...]

Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[Daniel Borkmann]

On 9/16/25 5:17 PM, Paul Chaignon wrote:
> Syzkaller found a kernel warning on the following sock_addr program:
> 
>      0: r0 = 0
>      1: r2 = *(u32 *)(r1 +60)
>      2: exit
> 
> which triggers:
> 
>      verifier bug: error during ctx access conversion (0)
> 
> This is happening because offset 60 in bpf_sock_addr corresponds to an
> implicit padding of 4 bytes, right after msg_src_ip4. Access to this
> padding isn't rejected in sock_addr_is_valid_access and it thus later
> fails to convert the access.
> 
> This patch fixes it by explicitly checking the various fields of
> bpf_sock_addr in sock_addr_is_valid_access.
> 
> I checked the other ctx structures and is_valid_access functions and
> didn't find any other similar cases. Other cases of (properly handled)
> padding are covered in new tests in a subsequent patch.
> 
> Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
> Reported-by: syzbot+136ca59d411f92e821b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=136ca59d411f92e821b7
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
>   net/core/filter.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index da391e2b0788..9ac58960e59e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -9284,13 +9284,19 @@ static bool sock_addr_is_valid_access(int off, int size,
>   			return false;
>   		info->reg_type = PTR_TO_SOCKET;
>   		break;
> -	default:
> +	case bpf_ctx_range(struct bpf_sock_addr, user_family):
> +	case bpf_ctx_range(struct bpf_sock_addr, family):
> +	case bpf_ctx_range(struct bpf_sock_addr, type):
> +	case bpf_ctx_range(struct bpf_sock_addr, protocol):
>   		if (type == BPF_READ) {
>   			if (size != size_default)
>   				return false;
>   		} else {
>   			return false;
>   		}
> +		break;

Looks good to me, we can probably also simplify this a bit into:

                 if (type != BPF_READ)
                         return false;
                 if (size != size_default)
                         return false;
                 break;

Also, targeting bpf-next tree seems okay here since the verifier
can gracefully handle and reject such program (and WARN_ONCE is
only triggered on CONFIG_DEBUG_KERNEL)?

> +	default:
> +		return false;
>   	}
>   
>   	return true;

Thanks,
Daniel

Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[Paul Chaignon]

On Tue, Sep 16, 2025 at 09:44:28PM +0200, Daniel Borkmann wrote:
> On 9/16/25 5:17 PM, Paul Chaignon wrote:
> > Syzkaller found a kernel warning on the following sock_addr program:
> > 
> >      0: r0 = 0
> >      1: r2 = *(u32 *)(r1 +60)
> >      2: exit
> > 
> > which triggers:
> > 
> >      verifier bug: error during ctx access conversion (0)
> > 
> > This is happening because offset 60 in bpf_sock_addr corresponds to an
> > implicit padding of 4 bytes, right after msg_src_ip4. Access to this
> > padding isn't rejected in sock_addr_is_valid_access and it thus later
> > fails to convert the access.
> > 
> > This patch fixes it by explicitly checking the various fields of
> > bpf_sock_addr in sock_addr_is_valid_access.
> > 
> > I checked the other ctx structures and is_valid_access functions and
> > didn't find any other similar cases. Other cases of (properly handled)
> > padding are covered in new tests in a subsequent patch.
> > 
> > Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
> > Reported-by: syzbot+136ca59d411f92e821b7@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=136ca59d411f92e821b7
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> >   net/core/filter.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index da391e2b0788..9ac58960e59e 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -9284,13 +9284,19 @@ static bool sock_addr_is_valid_access(int off, int size,
> >   			return false;
> >   		info->reg_type = PTR_TO_SOCKET;
> >   		break;
> > -	default:
> > +	case bpf_ctx_range(struct bpf_sock_addr, user_family):
> > +	case bpf_ctx_range(struct bpf_sock_addr, family):
> > +	case bpf_ctx_range(struct bpf_sock_addr, type):
> > +	case bpf_ctx_range(struct bpf_sock_addr, protocol):
> >   		if (type == BPF_READ) {
> >   			if (size != size_default)
> >   				return false;
> >   		} else {
> >   			return false;
> >   		}
> > +		break;
> 
> Looks good to me, we can probably also simplify this a bit into:

Thanks for the review!

> 
>                 if (type != BPF_READ)
>                         return false;
>                 if (size != size_default)
>                         return false;
>                 break;
> 
> Also, targeting bpf-next tree seems okay here since the verifier
> can gracefully handle and reject such program (and WARN_ONCE is
> only triggered on CONFIG_DEBUG_KERNEL)?

Yep, that makes sense. I did not think this through :')

> 
> > +	default:
> > +		return false;
> >   	}
> >   	return true;
> 
> Thanks,
> Daniel

Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[Eduard Zingerman]

On Tue, 2025-09-16 at 17:17 +0200, Paul Chaignon wrote:
> Syzkaller found a kernel warning on the following sock_addr program:
> 
>     0: r0 = 0
>     1: r2 = *(u32 *)(r1 +60)
>     2: exit
> 
> which triggers:
> 
>     verifier bug: error during ctx access conversion (0)
> 
> This is happening because offset 60 in bpf_sock_addr corresponds to an
> implicit padding of 4 bytes, right after msg_src_ip4. Access to this
> padding isn't rejected in sock_addr_is_valid_access and it thus later
> fails to convert the access.
> 
> This patch fixes it by explicitly checking the various fields of
> bpf_sock_addr in sock_addr_is_valid_access.
> 
> I checked the other ctx structures and is_valid_access functions and
> didn't find any other similar cases. Other cases of (properly handled)
> padding are covered in new tests in a subsequent patch.
> 
> Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
> Reported-by: syzbot+136ca59d411f92e821b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=136ca59d411f92e821b7
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

Double checked other context types with holes and paddings:
- bpf_sk_lookup
- bpf_sock
- __sk_buff
- sk_reuseport_md

And agree with Paul's conclusion.
(Note, however, that bpf_sock and __sk_buff explicitly refer to
 padding offsets).

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr

[Paul Chaignon]

On Tue, Sep 16, 2025 at 03:45:51PM -0700, Eduard Zingerman wrote:
> On Tue, 2025-09-16 at 17:17 +0200, Paul Chaignon wrote:
> > Syzkaller found a kernel warning on the following sock_addr program:
> > 
> >     0: r0 = 0
> >     1: r2 = *(u32 *)(r1 +60)
> >     2: exit
> > 
> > which triggers:
> > 
> >     verifier bug: error during ctx access conversion (0)
> > 
> > This is happening because offset 60 in bpf_sock_addr corresponds to an
> > implicit padding of 4 bytes, right after msg_src_ip4. Access to this
> > padding isn't rejected in sock_addr_is_valid_access and it thus later
> > fails to convert the access.
> > 
> > This patch fixes it by explicitly checking the various fields of
> > bpf_sock_addr in sock_addr_is_valid_access.
> > 
> > I checked the other ctx structures and is_valid_access functions and
> > didn't find any other similar cases. Other cases of (properly handled)
> > padding are covered in new tests in a subsequent patch.
> > 
> > Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
> > Reported-by: syzbot+136ca59d411f92e821b7@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=136ca59d411f92e821b7
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> 
> Double checked other context types with holes and paddings:
> - bpf_sk_lookup
> - bpf_sock
> - __sk_buff
> - sk_reuseport_md
> 
> And agree with Paul's conclusion.
> (Note, however, that bpf_sock and __sk_buff explicitly refer to
>  padding offsets).

Thanks for double checking! I think I had missed sk_reuseport_md when
going through those. I had also forgotten the test case for bpf_sock.
I added both as selftests in the v2.

> 
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> 
> [...]