bpf: fix NULL pointer dereference in print_reg_state()

[PATCH v2] bpf: fix NULL pointer dereference in print_reg_state()

Posted by Brahmajit Das 1 week, 1 day ago

Syzkaller reported a general protection fault due to a NULL pointer
dereference in print_reg_state() when accessing reg->map_ptr without
checking if it is NULL.

The existing code assumes reg->map_ptr is always valid before
dereferencing reg->map_ptr->name, reg->map_ptr->key_size, and
reg->map_ptr->value_size.

Fix this by adding explicit NULL checks before accessing reg->map_ptr
and its members. This prevents crashes when reg->map_ptr is NULL,
improving the robustness of the BPF verifier's verbose logging.

Reported-by: syzbot+d36d5ae81e1b0a53ef58@syzkaller.appspotmail.com
Signed-off-by: Brahmajit Das <listout@listout.xyz>
---
 kernel/bpf/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index f50533169cc3..5ffb8d778b92 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -704,7 +704,7 @@ static void print_reg_state(struct bpf_verifier_env *env,
 		verbose_a("ref_obj_id=%d", reg->ref_obj_id);
 	if (type_is_non_owning_ref(reg->type))
 		verbose_a("%s", "non_own_ref");
-	if (type_is_map_ptr(t)) {
+	if (type_is_map_ptr(t) && reg->map_ptr) {
 		if (reg->map_ptr->name[0])
 			verbose_a("map=%s", reg->map_ptr->name);
 		verbose_a("ks=%d,vs=%d",
-- 
2.51.0

Re: [PATCH v2] bpf: fix NULL pointer dereference in print_reg_state()

Posted by Alexei Starovoitov 1 week ago

On Wed, Sep 24, 2025 at 1:43 AM Brahmajit Das <listout@listout.xyz> wrote:
>
> Syzkaller reported a general protection fault due to a NULL pointer
> dereference in print_reg_state() when accessing reg->map_ptr without
> checking if it is NULL.
>
> The existing code assumes reg->map_ptr is always valid before
> dereferencing reg->map_ptr->name, reg->map_ptr->key_size, and
> reg->map_ptr->value_size.
>
> Fix this by adding explicit NULL checks before accessing reg->map_ptr
> and its members. This prevents crashes when reg->map_ptr is NULL,
> improving the robustness of the BPF verifier's verbose logging.
>
> Reported-by: syzbot+d36d5ae81e1b0a53ef58@syzkaller.appspotmail.com
> Signed-off-by: Brahmajit Das <listout@listout.xyz>
> ---
>  kernel/bpf/log.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
> index f50533169cc3..5ffb8d778b92 100644
> --- a/kernel/bpf/log.c
> +++ b/kernel/bpf/log.c
> @@ -704,7 +704,7 @@ static void print_reg_state(struct bpf_verifier_env *env,
>                 verbose_a("ref_obj_id=%d", reg->ref_obj_id);
>         if (type_is_non_owning_ref(reg->type))
>                 verbose_a("%s", "non_own_ref");
> -       if (type_is_map_ptr(t)) {
> +       if (type_is_map_ptr(t) && reg->map_ptr) {

You ignored earlier feedback.
Fix the root cause, not the symptom.

pw-bot: cr

Re: [PATCH v2] bpf: fix NULL pointer dereference in print_reg_state()

Posted by Brahmajit Das 1 week ago

On 24.09.2025 09:32, Alexei Starovoitov wrote:
> On Wed, Sep 24, 2025 at 1:43 AM Brahmajit Das <listout@listout.xyz> wrote:
> >
> > Syzkaller reported a general protection fault due to a NULL pointer
> > dereference in print_reg_state() when accessing reg->map_ptr without
> > checking if it is NULL.
> >
...snip...
> > -       if (type_is_map_ptr(t)) {
> > +       if (type_is_map_ptr(t) && reg->map_ptr) {
> 
> You ignored earlier feedback.
> Fix the root cause, not the symptom.
> 
> pw-bot: cr

I'm not sure if I'm headed the write direction but it seems like in
check_alu_op, we are calling adjust_scalar_min_max_vals when we get an
BPF_NEG as opcode. Which has a call to __mark_reg_known when opcode is
BPF_NEG. And __mark_reg_known clears map_ptr with

	/* Clear off and union(map_ptr, range) */
	memset(((u8 *)reg) + sizeof(reg->type), 0,
	       offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));

-- 
Regards,
listout

Re: [PATCH v2] bpf: fix NULL pointer dereference in print_reg_state()

Posted by Alexei Starovoitov 1 week ago

On Wed, Sep 24, 2025 at 4:41 PM Brahmajit Das <listout@listout.xyz> wrote:
>
> On 24.09.2025 09:32, Alexei Starovoitov wrote:
> > On Wed, Sep 24, 2025 at 1:43 AM Brahmajit Das <listout@listout.xyz> wrote:
> > >
> > > Syzkaller reported a general protection fault due to a NULL pointer
> > > dereference in print_reg_state() when accessing reg->map_ptr without
> > > checking if it is NULL.
> > >
> ...snip...
> > > -       if (type_is_map_ptr(t)) {
> > > +       if (type_is_map_ptr(t) && reg->map_ptr) {
> >
> > You ignored earlier feedback.
> > Fix the root cause, not the symptom.
> >
> > pw-bot: cr
>
> I'm not sure if I'm headed the write direction but it seems like in
> check_alu_op, we are calling adjust_scalar_min_max_vals when we get an
> BPF_NEG as opcode. Which has a call to __mark_reg_known when opcode is
> BPF_NEG. And __mark_reg_known clears map_ptr with

Looks like we're getting somewhere.
It seems the verifier is not clearing reg->type.
adjust_scalar_min_max_vals() should be called on scalar types only.

Re: [PATCH v2] bpf: fix NULL pointer dereference in print_reg_state()

Posted by Brahmajit Das 1 week ago

On 24.09.2025 09:32, Alexei Starovoitov wrote:
> On Wed, Sep 24, 2025 at 1:43 AM Brahmajit Das <listout@listout.xyz> wrote:
> >
> > Syzkaller reported a general protection fault due to a NULL pointer
> > dereference in print_reg_state() when accessing reg->map_ptr without
> > checking if it is NULL.
> >
> > The existing code assumes reg->map_ptr is always valid before
> > dereferencing reg->map_ptr->name, reg->map_ptr->key_size, and
> > reg->map_ptr->value_size.
> >
> > Fix this by adding explicit NULL checks before accessing reg->map_ptr
> > and its members. This prevents crashes when reg->map_ptr is NULL,
> > improving the robustness of the BPF verifier's verbose logging.
> >
> > Reported-by: syzbot+d36d5ae81e1b0a53ef58@syzkaller.appspotmail.com
> > Signed-off-by: Brahmajit Das <listout@listout.xyz>
> > ---
> >  kernel/bpf/log.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
> > index f50533169cc3..5ffb8d778b92 100644
> > --- a/kernel/bpf/log.c
> > +++ b/kernel/bpf/log.c
> > @@ -704,7 +704,7 @@ static void print_reg_state(struct bpf_verifier_env *env,
> >                 verbose_a("ref_obj_id=%d", reg->ref_obj_id);
> >         if (type_is_non_owning_ref(reg->type))
> >                 verbose_a("%s", "non_own_ref");
> > -       if (type_is_map_ptr(t)) {
> > +       if (type_is_map_ptr(t) && reg->map_ptr) {
> 
> You ignored earlier feedback.
> Fix the root cause, not the symptom.
> 
> pw-bot: cr

Alexei, I did not, the patches (v1 and v2) were sent in a very short
timeframe, when you gave me the feedback I had already sent the v2 so
your feedback applies to v2 as well :)

I'm working on fixing/understanding the issue. I went one function lower
from where print_reg_state is being called and added a few debugging
statements like this

--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -758,6 +758,12 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifie
                        continue;
                if (!print_all && !reg_scratched(env, i))
                        continue;
+               pr_err("&state->regs[%d] = %p\n", i, (void *)&state->regs[i]);
+               pr_err("reg               = %p\n", (void *)reg);
+               pr_err("&reg->map_ptr      = %p\n", (void *)&reg->map_ptr);
+               pr_err("&state->regs[%d].map_ptr = %p\n", i, (void *)&state->regs[i].map_ptr);
+               pr_err("state->regs[%d].map_ptr is NULL %d\n", i, state->regs[i].map_ptr == NULL);
+               pr_err("regs->map_ptr is NULL %d\n", reg->map_ptr == NULL);
                verbose(env, " R%d", i);
                verbose(env, "=");
                print_reg_state(env, state, reg);

Both reg->map_ptr and state->regs[i].map_ptr reports map_ptr is NULL.
For now I'm bit stuck and trying to understand why that would be.
I got the reproducer from
https://syzkaller.appspot.com/text?tag=ReproC&x=1608c27c580000

-- 
Regards,
listout