Cgroup link updates compare only the program type. Programs verified
for different hooks can share a type, so a UDP6 sock_addr program can
replace a UDP4 program and write beyond the four-byte ipc.addr field in
the stack-local struct ipcm_cookie. The same gap permits incompatible
LSM_MAC and LSM_CGROUP replacements.
Validate every replacement program against the link attach type before
dispatching to the link-specific update operation. Keep the CGROUP_SKB
CAP_NET_ADMIN check at initial attach so pinned or delegated link FDs can
still be updated after a capability drop.
Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active
bpf_cgroup_link")
Signed-off-by: Sanghyun Park <[email protected]>
---
v4:
- Validate every program link update through the shared attach-type helper.
- Fold the LSM attach-flavor check into that helper.
- Keep the CGROUP_SKB permission check in the attach-only wrapper and rename
the wrapper to describe that role.
- Follow the preferred BPF multi-line comment style.
- Add focused regression selftests as patch 2/2.
v3:
https://lore.kernel.org/r/[email protected]
- Factor the CGROUP_SKB CAP_NET_ADMIN check into an attach-only helper.
v2:
https://lore.kernel.org/r/[email protected]
- Extend validation from cgroup sock_addr programs to all cgroup program
types, including exact LSM attach flavors.
- Preserve legacy CGROUP_SKB ingress/egress replacement compatibility.
- Keep the CGROUP_SKB CAP_NET_ADMIN check on attach, not link update.
v1:
https://lore.kernel.org/r/[email protected]
---
kernel/bpf/syscall.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6874ba1424af..84823149dffa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4481,14 +4481,9 @@ static int bpf_prog_attach_check_attach_type(const
struct bpf_prog *prog,
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
case BPF_PROG_TYPE_SK_LOOKUP:
+ case BPF_PROG_TYPE_LSM:
return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
case BPF_PROG_TYPE_CGROUP_SKB:
- if (!bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
- /* cg-skb progs can be loaded by unpriv user.
- * check permissions at attach time.
- */
- return -EPERM;
-
ptype = attach_type_to_prog_type(attach_type);
if (prog->type != ptype)
return -EINVAL;
@@ -4542,6 +4537,20 @@ static int bpf_prog_attach_check_attach_type(const
struct bpf_prog *prog,
}
}
+static int bpf_prog_attach_check_perm(const struct bpf_prog *prog,
+ enum bpf_attach_type attach_type)
+{
+ /*
+ * CGROUP_SKB programs can be loaded by unprivileged users, so check
+ * permissions at attach time.
+ */
+ if (prog->type == BPF_PROG_TYPE_CGROUP_SKB &&
+ !bpf_token_capable(prog->aux->token, CAP_NET_ADMIN))
+ return -EPERM;
+
+ return bpf_prog_attach_check_attach_type(prog, attach_type);
+}
+
static bool is_cgroup_prog_type(enum bpf_prog_type ptype, enum bpf_attach_type
atype,
bool check_atype)
{
@@ -4606,7 +4615,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
if (IS_ERR(prog))
return PTR_ERR(prog);
- if (bpf_prog_attach_check_attach_type(prog, attr->attach_type)) {
+ if (bpf_prog_attach_check_perm(prog, attr->attach_type)) {
bpf_prog_put(prog);
return -EINVAL;
}
@@ -5805,8 +5814,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t
uattr)
if (IS_ERR(prog))
return PTR_ERR(prog);
- ret = bpf_prog_attach_check_attach_type(prog,
- attr->link_create.attach_type);
+ ret = bpf_prog_attach_check_perm(prog,
+ attr->link_create.attach_type);
if (ret)
goto out;
@@ -5968,6 +5977,10 @@ static int link_update(union bpf_attr *attr)
goto out_put_progs;
}
+ ret = bpf_prog_attach_check_attach_type(new_prog, link->attach_type);
+ if (ret)
+ goto out_put_progs;
+
if (link->ops->update_prog)
ret = link->ops->update_prog(link, new_prog, old_prog);
else
--
2.48.1
