[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7c5553d33d0796a22ffa3d4c7e1791e7f033d43d.camel@gmail.com>
Date: Mon, 29 Apr 2024 12:34:06 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: Stanislav Fomichev <sdf@...gle.com>, bpf@...r.kernel.org,
netdev@...r.kernel.org
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
martin.lau@...ux.dev, song@...nel.org, yhs@...com,
john.fastabend@...il.com, kpsingh@...nel.org, haoluo@...gle.com,
jolsa@...nel.org, syzbot+838346b979830606c854@...kaller.appspotmail.com
Subject: Re: [PATCH bpf 1/3] bpf: Add BPF_PROG_TYPE_CGROUP_SKB attach type
enforcement in BPF_LINK_CREATE
On Fri, 2024-04-26 at 16:16 -0700, Stanislav Fomichev wrote:
> bpf_prog_attach uses attach_type_to_prog_type to enforce proper
> attach type for BPF_PROG_TYPE_CGROUP_SKB. link_create uses
> bpf_prog_get and relies on bpf_prog_attach_check_attach_type
> to properly verify prog_type <> attach_type association.
>
> Add missing attach_type enforcement for the link_create case.
> Otherwise, it's currently possible to attach cgroup_skb prog
> types to other cgroup hooks.
>
> Fixes: af6eea57437a ("bpf: Implement bpf_link-based cgroup BPF program attachment")
> Link: https://lore.kernel.org/bpf/0000000000004792a90615a1dde0@google.com/
> Reported-by: syzbot+838346b979830606c854@...kaller.appspotmail.com
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> ---
I've spent some time comparing:
- syscall.c:bpf_prog_attach()
- syscall.c:link_create()
- syscall.c:bpf_prog_attach_check_attach_type()
- syscall.c:attach_type_to_prog_type()
- verifier.c:check_return_code()
And it looks like BPF_PROG_TYPE_CGROUP_SKB is the only thing with
missing attach type checks.
Acked-by: Eduard Zingerman <eddyz87@...il.com>
(The interplay between the above functions seems a bit messy,
but I don't have good suggestions for refactoring at the moment.
It appears that bpf_prog_attach_check_attach_type() could be simplified
if prog->enforce_expected_attach_type would be set more aggressively and
if (prog->enforce_expected_attach_type &&
prog->expected_attach_type != attach_type)
return -EINVAL;
moved as a top-level check outside of the switch.
Also BPF_PROG_TYPE_SCHED_CLS case could be removed as it is handled
by default branch. But these are larger changes).
Powered by blists - more mailing lists