| Message ID | 20220107183049.311134-1-toke@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf,1/2] xdp: check prog type before updating BPF link | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 7 this patch: 7 |
| netdev/cc_maintainers | success | CCed 13 of 13 maintainers |
| netdev/build_clang | success | Errors and warnings before: 20 this patch: 20 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9 this patch: 9 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 12 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
| bpf/vmtest-bpf | fail | VM_Test |
| bpf/vmtest-bpf-PR | fail | PR summary |
On Fri, Jan 7, 2022 at 10:31 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > The bpf_xdp_link_update() function didn't check the program type before > updating the program, which made it possible to install any program type as > an XDP program, which is obviously not good. Syzbot managed to trigger this > by swapping in an LWT program on the XDP hook which would crash in a helper > call. > > Fix this by adding a check and bailing out if the types don't match. > > Fixes: 026a4c28e1db ("bpf, xdp: Implement LINK_UPDATE for BPF XDP link") > Reported-by: syzbot+983941aa85af6ded1fd9@syzkaller.appspotmail.com > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- The fix looks good to me, thanks. I'd love it if this was done generically in link_update, but each link type has its own locking schema for link->prog, so I didn't figure out a way to do this in a centralized way. Acked-by: Andrii Nakryiko <andrii@kernel.org> > net/core/dev.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/core/dev.c b/net/core/dev.c > index c4708e2487fb..2078d04c6482 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -9656,6 +9656,12 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog, > goto out_unlock; > } > old_prog = link->prog; > + if (old_prog->type != new_prog->type || > + old_prog->expected_attach_type != new_prog->expected_attach_type) { > + err = -EINVAL; > + goto out_unlock; > + } > + > if (old_prog == new_prog) { > /* no-op, don't disturb drivers */ > bpf_prog_put(new_prog); > -- > 2.34.1 >
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Fri, Jan 7, 2022 at 10:31 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> The bpf_xdp_link_update() function didn't check the program type before >> updating the program, which made it possible to install any program type as >> an XDP program, which is obviously not good. Syzbot managed to trigger this >> by swapping in an LWT program on the XDP hook which would crash in a helper >> call. >> >> Fix this by adding a check and bailing out if the types don't match. >> >> Fixes: 026a4c28e1db ("bpf, xdp: Implement LINK_UPDATE for BPF XDP link") >> Reported-by: syzbot+983941aa85af6ded1fd9@syzkaller.appspotmail.com >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- > > The fix looks good to me, thanks. I'd love it if this was done > generically in link_update, but each link type has its own locking > schema for link->prog, so I didn't figure out a way to do this in a > centralized way. Yeah, moving it to be a generic check was my first thought as well, but I came to the same conclusion :) > Acked-by: Andrii Nakryiko <andrii@kernel.org> Thanks! -Toke
diff --git a/net/core/dev.c b/net/core/dev.c index c4708e2487fb..2078d04c6482 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9656,6 +9656,12 @@ static int bpf_xdp_link_update(struct bpf_link *link, struct bpf_prog *new_prog, goto out_unlock; } old_prog = link->prog; + if (old_prog->type != new_prog->type || + old_prog->expected_attach_type != new_prog->expected_attach_type) { + err = -EINVAL; + goto out_unlock; + } + if (old_prog == new_prog) { /* no-op, don't disturb drivers */ bpf_prog_put(new_prog);
The bpf_xdp_link_update() function didn't check the program type before updating the program, which made it possible to install any program type as an XDP program, which is obviously not good. Syzbot managed to trigger this by swapping in an LWT program on the XDP hook which would crash in a helper call. Fix this by adding a check and bailing out if the types don't match. Fixes: 026a4c28e1db ("bpf, xdp: Implement LINK_UPDATE for BPF XDP link") Reported-by: syzbot+983941aa85af6ded1fd9@syzkaller.appspotmail.com Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- net/core/dev.c | 6 ++++++ 1 file changed, 6 insertions(+)