2020-07-14 00:43:02

by Peilin Ye

Subject: [Linux-kernel-mentees] [PATCH] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
as NULL. This patch fixes the following syzbot bug:

https://syzkaller.appspot.com/bug?id=5edd146856fd513747c1992442732e5a0e9ba355

Reported-by: [email protected]
Signed-off-by: Peilin Ye <[email protected]>
---
kernel/bpf/btf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 30721f2c2d10..3e981b183fa4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4088,7 +4088,7 @@ static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
const char *tname, *sym;
u32 btf_id, i;

- if (IS_ERR(btf_vmlinux)) {
+ if (IS_ERR_OR_NULL(btf_vmlinux)) {
bpf_log(log, "btf_vmlinux is malformed\n");
return -EINVAL;
}
--
2.25.1

2020-07-14 00:56:36

by Andrii Nakryiko

Subject: Re: [Linux-kernel-mentees] [PATCH] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

On Mon, Jul 13, 2020 at 5:43 PM Peilin Ye <[email protected]> wrote:
>
> Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
> as NULL. This patch fixes the following syzbot bug:
>
> https://syzkaller.appspot.com/bug?id=5edd146856fd513747c1992442732e5a0e9ba355
>
> Reported-by: [email protected]
> Signed-off-by: Peilin Ye <[email protected]>
> ---
> kernel/bpf/btf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 30721f2c2d10..3e981b183fa4 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4088,7 +4088,7 @@ static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
> const char *tname, *sym;
> u32 btf_id, i;
>
> - if (IS_ERR(btf_vmlinux)) {
> + if (IS_ERR_OR_NULL(btf_vmlinux)) {
> bpf_log(log, "btf_vmlinux is malformed\n");

Can you please split IS_ERR and NULL cases and emit different messages
to log? If the kernel is not built with btf_vmlinux, saying that it's
"malformed" will confuse people. Thanks.

> return -EINVAL;
> }
> --
> 2.25.1
>

2020-07-14 01:30:02

by Peilin Ye

Subject: [Linux-kernel-mentees] [PATCH v2] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
as NULL. This patch fixes the following syzbot bug:

https://syzkaller.appspot.com/bug?id=5edd146856fd513747c1992442732e5a0e9ba355

Reported-by: [email protected]
Signed-off-by: Peilin Ye <[email protected]>
---
Thank you for reviewing my patch! I am new to Linux kernel development; would
the log message and errno be appropriate for this case?

Change in v2:
- Split NULL and IS_ERR cases.

kernel/bpf/btf.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 30721f2c2d10..092116a311f4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4088,6 +4088,11 @@ static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
const char *tname, *sym;
u32 btf_id, i;

+ if (!btf_vmlinux) {
+ bpf_log(log, "btf_vmlinux doesn't exist\n");
+ return -EINVAL;
+ }
+
if (IS_ERR(btf_vmlinux)) {
bpf_log(log, "btf_vmlinux is malformed\n");
return -EINVAL;
--
2.25.1

2020-07-14 04:40:58

by Andrii Nakryiko

Subject: Re: [Linux-kernel-mentees] [PATCH v2] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

On Mon, Jul 13, 2020 at 6:29 PM Peilin Ye <[email protected]> wrote:
>
> Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
> as NULL. This patch fixes the following syzbot bug:
>
> https://syzkaller.appspot.com/bug?id=5edd146856fd513747c1992442732e5a0e9ba355
>
> Reported-by: [email protected]
> Signed-off-by: Peilin Ye <[email protected]>
> ---
> Thank you for reviewing my patch! I am new to Linux kernel development; would
> the log message and errno be appropriate for this case?

I think it's good enough, thanks for the fix.

Acked-by: Andrii Nakryiko <[email protected]>

>
> Change in v2:
> - Split NULL and IS_ERR cases.
>
> kernel/bpf/btf.c | 5 +++++
> 1 file changed, 5 insertions(+)
>

[...]

2020-07-14 17:30:55

by Alexei Starovoitov

Subject: Re: [Linux-kernel-mentees] [PATCH v2] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

On Mon, Jul 13, 2020 at 9:38 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Mon, Jul 13, 2020 at 6:29 PM Peilin Ye <[email protected]> wrote:
> >
> > Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
> > as NULL. This patch fixes the following syzbot bug:
> >
> > https://syzkaller.appspot.com/bug?id=5edd146856fd513747c1992442732e5a0e9ba355

The link looks wrong?
Nothing in the stack trace indicates this issue.

> > Reported-by: [email protected]
> > Signed-off-by: Peilin Ye <[email protected]>
> > ---
> > Thank you for reviewing my patch! I am new to Linux kernel development; would
> > the log message and errno be appropriate for this case?
>
> I think it's good enough, thanks for the fix.
>
> Acked-by: Andrii Nakryiko <[email protected]>
>
> >
> > Change in v2:
> > - Split NULL and IS_ERR cases.
> >
> > kernel/bpf/btf.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
>
> [...]

2020-07-14 18:11:02

by Peilin Ye

Subject: [Linux-kernel-mentees] [PATCH v3] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
as NULL. This patch fixes the following syzbot bug:

https://syzkaller.appspot.com/bug?id=f823224ada908fa5c207902a5a62065e53ca0fcc

Reported-by: [email protected]
Signed-off-by: Peilin Ye <[email protected]>
---
Sorry, I got the link wrong. Thank you for pointing that out.

Change in v3:
- Fix incorrect syzbot dashboard link.

Change in v2:
- Split NULL and IS_ERR cases.

kernel/bpf/btf.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 30721f2c2d10..092116a311f4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4088,6 +4088,11 @@ static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
const char *tname, *sym;
u32 btf_id, i;

+ if (!btf_vmlinux) {
+ bpf_log(log, "btf_vmlinux doesn't exist\n");
+ return -EINVAL;
+ }
+
if (IS_ERR(btf_vmlinux)) {
bpf_log(log, "btf_vmlinux is malformed\n");
return -EINVAL;
--
2.25.1

2020-07-15 21:07:59

by Daniel Borkmann

Subject: Re: [Linux-kernel-mentees] [PATCH v3] bpf: Fix NULL pointer dereference in __btf_resolve_helper_id()

On 7/14/20 8:09 PM, Peilin Ye wrote:
> Prevent __btf_resolve_helper_id() from dereferencing `btf_vmlinux`
> as NULL. This patch fixes the following syzbot bug:
>
> https://syzkaller.appspot.com/bug?id=f823224ada908fa5c207902a5a62065e53ca0fcc
>
> Reported-by: [email protected]
> Signed-off-by: Peilin Ye <[email protected]>

Looks good, applied, thanks! As far as I can tell all the other occurrences are
gated behind btf_parse_vmlinux() where we also init struct_opts, etc.

So for bpf-next this would then end up looking like ...

int btf_resolve_helper_id(struct bpf_verifier_log *log,
const struct bpf_func_proto *fn, int arg)
{
int id;

if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)
return -EINVAL;
id = fn->btf_id[arg];
if (!id || !btf_vmlinux || id > btf_vmlinux->nr_types)
return -EINVAL;
return id;
}

> ---
> Sorry, I got the link wrong. Thank you for pointing that out.
>
> Change in v3:
> - Fix incorrect syzbot dashboard link.
>
> Change in v2:
> - Split NULL and IS_ERR cases.
>
> kernel/bpf/btf.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 30721f2c2d10..092116a311f4 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4088,6 +4088,11 @@ static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
> const char *tname, *sym;
> u32 btf_id, i;
>
> + if (!btf_vmlinux) {
> + bpf_log(log, "btf_vmlinux doesn't exist\n");
> + return -EINVAL;
> + }
> +
> if (IS_ERR(btf_vmlinux)) {
> bpf_log(log, "btf_vmlinux is malformed\n");
> return -EINVAL;
>