* [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr
@ 2025-09-16 15:17 Paul Chaignon
2025-09-16 15:18 ` [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h Paul Chaignon
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Paul Chaignon @ 2025-09-16 15:17 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h
2025-09-16 15:17 [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Paul Chaignon
@ 2025-09-16 15:18 ` Paul Chaignon
2025-09-16 22:53 ` Eduard Zingerman
2025-09-16 15:19 ` [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding Paul Chaignon
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Paul Chaignon @ 2025-09-16 15:18 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding
2025-09-16 15:17 [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Paul Chaignon
2025-09-16 15:18 ` [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h Paul Chaignon
@ 2025-09-16 15:19 ` Paul Chaignon
2025-09-16 22:59 ` Eduard Zingerman
2025-09-16 19:44 ` [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Daniel Borkmann
2025-09-16 22:45 ` Eduard Zingerman
3 siblings, 1 reply; 9+ messages in thread
From: Paul Chaignon @ 2025-09-16 15:19 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr
2025-09-16 15:17 [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Paul Chaignon
2025-09-16 15:18 ` [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h Paul Chaignon
2025-09-16 15:19 ` [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding Paul Chaignon
@ 2025-09-16 19:44 ` Daniel Borkmann
2025-09-17 7:51 ` Paul Chaignon
2025-09-16 22:45 ` Eduard Zingerman
3 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2025-09-16 19:44 UTC (permalink / raw)
To: Paul Chaignon, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr
2025-09-16 15:17 [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Paul Chaignon
` (2 preceding siblings ...)
2025-09-16 19:44 ` [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Daniel Borkmann
@ 2025-09-16 22:45 ` Eduard Zingerman
2025-09-17 8:02 ` Paul Chaignon
3 siblings, 1 reply; 9+ messages in thread
From: Eduard Zingerman @ 2025-09-16 22:45 UTC (permalink / raw)
To: Paul Chaignon, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau
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>
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h
2025-09-16 15:18 ` [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h Paul Chaignon
@ 2025-09-16 22:53 ` Eduard Zingerman
0 siblings, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2025-09-16 22:53 UTC (permalink / raw)
To: Paul Chaignon, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau
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
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding
2025-09-16 15:19 ` [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding Paul Chaignon
@ 2025-09-16 22:59 ` Eduard Zingerman
0 siblings, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2025-09-16 22:59 UTC (permalink / raw)
To: Paul Chaignon, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau
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); \
> + }
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr
2025-09-16 19:44 ` [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Daniel Borkmann
@ 2025-09-17 7:51 ` Paul Chaignon
0 siblings, 0 replies; 9+ messages in thread
From: Paul Chaignon @ 2025-09-17 7:51 UTC (permalink / raw)
To: Daniel Borkmann
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr
2025-09-16 22:45 ` Eduard Zingerman
@ 2025-09-17 8:02 ` Paul Chaignon
0 siblings, 0 replies; 9+ messages in thread
From: Paul Chaignon @ 2025-09-17 8:02 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau
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>
>
> [...]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-17 8:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-16 15:17 [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Paul Chaignon
2025-09-16 15:18 ` [PATCH bpf 2/3] selftests/bpf: Move macros to bpf_misc.h Paul Chaignon
2025-09-16 22:53 ` Eduard Zingerman
2025-09-16 15:19 ` [PATCH bpf 3/3] selftest/bpf: Test accesses to ctx padding Paul Chaignon
2025-09-16 22:59 ` Eduard Zingerman
2025-09-16 19:44 ` [PATCH bpf 1/3] bpf: Explicitly check accesses to bpf_sock_addr Daniel Borkmann
2025-09-17 7:51 ` Paul Chaignon
2025-09-16 22:45 ` Eduard Zingerman
2025-09-17 8:02 ` Paul Chaignon
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).