[PATCH bpf v3 0/3] Fixes for bpf link update

[PATCH bpf v3 0/3] Fixes for bpf link update

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

patch 1: fix for UAF issue when reading bpf link info.
patch 2: fix for storage not restored when update_effective_progs failed.
patch 3: Fix storage null-ptr-deref after replacing prog in cgrp_bpf.

v3:
- Add new fix for UAF issue when reading bpf link info. (Sashiko)
- Add new fix for storage not restored when update_effective_progs
  failed. (Sashiko)

v2:
https://lore.kernel.org/bpf/20260717073343.958862-1-pulehui@huaweicloud.com
- Fix invalid access for in-place update when storage changed. (Sashiko)

v1:
https://lore.kernel.org/bpf/20260714014659.401063-1-pulehui@huaweicloud.com

Pu Lehui (3):
  bpf: Fix potential UAF when reading bpf link info
  bpf, cgroup: Fix storage not restored when update_effective_progs
    failed
  bpf, cgroup: Fix storage null-ptr-deref after replacing prog

 kernel/bpf/cgroup.c  | 40 +++++++++++++++++++++++++++++++++++++++-
 kernel/bpf/syscall.c | 27 +++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

In bpf_link_show_fdinfo and bpf_link_get_info_by_fd, link->prog is
accessed without holding any locks. If the prog is concurrently replaced
via bpf_link_update, the old prog can be freed, leading to a potential
UAF issue.

Fix this by holding both normal RCU and tasks trace RCU read locks
before dereferencing the prog, as BPF_LINK_TYPE_ITER support both
non-sleepable and sleepable progs.

Fixes: f9d041271cf4 ("bpf: Refactor bpf_link update handling")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 kernel/bpf/syscall.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6db306d23b47..2458c68146b9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3471,9 +3471,10 @@ static const char *bpf_link_type_strs[] = {
 static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
 {
 	const struct bpf_link *link = filp->private_data;
-	const struct bpf_prog *prog = link->prog;
+	const struct bpf_prog *prog;
 	enum bpf_link_type type = link->type;
 	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
+	u32 prog_id;
 
 	if (type < ARRAY_SIZE(bpf_link_type_strs) && bpf_link_type_strs[type]) {
 		if (link->type == BPF_LINK_TYPE_KPROBE_MULTI)
@@ -3490,13 +3491,23 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
 	}
 	seq_printf(m, "link_id:\t%u\n", link->id);
 
+	/* prog can be sleepable */
+	rcu_read_lock_trace();
+	rcu_read_lock();
+	prog = READ_ONCE(link->prog);
 	if (prog) {
 		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
+		prog_id = prog->aux->id;
+	}
+	rcu_read_unlock();
+	rcu_read_unlock_trace();
+
+	if (prog) {
 		seq_printf(m,
 			   "prog_tag:\t%s\n"
 			   "prog_id:\t%u\n",
 			   prog_tag,
-			   prog->aux->id);
+			   prog_id);
 	}
 	if (link->ops->show_fdinfo)
 		link->ops->show_fdinfo(link, m);
@@ -5535,6 +5546,7 @@ static int bpf_link_get_info_by_fd(struct file *file,
 {
 	struct bpf_link_info __user *uinfo = u64_to_user_ptr(attr->info.info);
 	struct bpf_link_info info;
+	const struct bpf_prog *prog;
 	u32 info_len = attr->info.info_len;
 	int err;
 
@@ -5549,8 +5561,15 @@ static int bpf_link_get_info_by_fd(struct file *file,
 
 	info.type = link->type;
 	info.id = link->id;
-	if (link->prog)
-		info.prog_id = link->prog->aux->id;
+
+	/* prog can be sleepable */
+	rcu_read_lock_trace();
+	rcu_read_lock();
+	prog = READ_ONCE(link->prog);
+	if (prog)
+		info.prog_id = prog->aux->id;
+	rcu_read_unlock();
+	rcu_read_unlock_trace();
 
 	if (link->ops->fill_link_info) {
 		err = link->ops->fill_link_info(link, &info);
-- 
2.34.1

Re: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] bpf_netns_link_update_prog checks link->prog locklessly, allowing a TOCTOU race that violates atomic replacement guarantees.
- [High] Concurrent BPF_LINK_UPDATE can race with bpf_mprog_link to cause a Use-After-Free when dereferencing link->prog->type.
--

--- Patch [1]: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info ---
commit 5b8e0ac8575ebc387f2ddc7dee211697bb704588
Author: Pu Lehui <pulehui@huawei.com>

bpf: Fix potential UAF when reading bpf link info

This commit addresses a potential Use-After-Free issue when accessing
link->prog without locks in bpf_link_show_fdinfo and bpf_link_get_info_by_fd.
It resolves the problem by acquiring both normal RCU and tasks trace RCU
read locks before dereferencing the program, preventing concurrent freeing
during bpf_link_update operations.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6db306d23b479..2458c68146b95 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3490,13 +3491,23 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
>  	}
>  	seq_printf(m, "link_id:\t%u\n", link->id);
>  
> +	/* prog can be sleepable */
> +	rcu_read_lock_trace();
> +	rcu_read_lock();
> +	prog = READ_ONCE(link->prog);

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing lockless 
dereferences of link->prog here, could a similar TOCTOU race happen in 
bpf_netns_link_update_prog()?

kernel/bpf/net_namespace.c:bpf_netns_link_update_prog() {
    ...
	if (old_prog && old_prog != link->prog)
		return -EPERM;
	if (new_prog->type != link->prog->type)
		return -EINVAL;

	mutex_lock(&netns_bpf_mutex);
    ...
}

Two threads concurrently calling BPF_LINK_UPDATE on a netns link could both 
evaluate the old_prog != link->prog check locklessly and pass it. If they 
acquire netns_bpf_mutex one after the other, would the second thread 
blindly overwrite link->prog, overriding the first thread's update and 
violating the BPF_F_REPLACE API contract?

[Severity: High]
This is also a pre-existing issue, but does a similar Use-After-Free race 
exist in bpf_mprog_link()?

kernel/bpf/mprog.c:bpf_mprog_link() {
    ...
	if (type && link->prog->type != type) {
		bpf_link_put(link);
		return -EINVAL;
	}
    ...
}

If a thread calls BPF_PROG_ATTACH on a tcx hook (holding the sleepable 
rtnl_lock) and supplies a relative_fd pointing to a netns link, 
bpf_mprog_link() executes without an RCU read-side critical section.

Could this thread be preempted, allowing a concurrent BPF_LINK_UPDATE on 
the same netns link to replace and free link->prog? When the first thread 
resumes, it would dereference link->prog->type where link->prog now points 
to freed memory.

>  	if (prog) {
>  		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> +		prog_id = prog->aux->id;
> +	}
> +	rcu_read_unlock();
> +	rcu_read_unlock_trace();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720033055.1215477-1-pulehui@huaweicloud.com?part=1

Re: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[Pu Lehui]

On 2026/7/20 11:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] bpf_netns_link_update_prog checks link->prog locklessly, allowing a TOCTOU race that violates atomic replacement guarantees.
> - [High] Concurrent BPF_LINK_UPDATE can race with bpf_mprog_link to cause a Use-After-Free when dereferencing link->prog->type.
> --
> 
> --- Patch [1]: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info ---
> commit 5b8e0ac8575ebc387f2ddc7dee211697bb704588
> Author: Pu Lehui <pulehui@huawei.com>
> 
> bpf: Fix potential UAF when reading bpf link info
> 
> This commit addresses a potential Use-After-Free issue when accessing
> link->prog without locks in bpf_link_show_fdinfo and bpf_link_get_info_by_fd.
> It resolves the problem by acquiring both normal RCU and tasks trace RCU
> read locks before dereferencing the program, preventing concurrent freeing
> during bpf_link_update operations.
> 
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 6db306d23b479..2458c68146b95 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -3490,13 +3491,23 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
>>   	}
>>   	seq_printf(m, "link_id:\t%u\n", link->id);
>>   
>> +	/* prog can be sleepable */
>> +	rcu_read_lock_trace();
>> +	rcu_read_lock();
>> +	prog = READ_ONCE(link->prog);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but since you are fixing lockless
> dereferences of link->prog here, could a similar TOCTOU race happen in
> bpf_netns_link_update_prog()?
> 
> kernel/bpf/net_namespace.c:bpf_netns_link_update_prog() {
>      ...
> 	if (old_prog && old_prog != link->prog)
> 		return -EPERM;
> 	if (new_prog->type != link->prog->type)
> 		return -EINVAL;
> 
> 	mutex_lock(&netns_bpf_mutex);
>      ...
> }

I will move forware the `mutex_lock(&netns_bpf_mutex)` in next.

> 
> Two threads concurrently calling BPF_LINK_UPDATE on a netns link could both
> evaluate the old_prog != link->prog check locklessly and pass it. If they
> acquire netns_bpf_mutex one after the other, would the second thread
> blindly overwrite link->prog, overriding the first thread's update and
> violating the BPF_F_REPLACE API contract?
> 
> [Severity: High]
> This is also a pre-existing issue, but does a similar Use-After-Free race
> exist in bpf_mprog_link()?
> 
> kernel/bpf/mprog.c:bpf_mprog_link() {
>      ...
> 	if (type && link->prog->type != type) {
> 		bpf_link_put(link);
> 		return -EINVAL;
> 	}
>      ...
> }
> 
> If a thread calls BPF_PROG_ATTACH on a tcx hook (holding the sleepable
> rtnl_lock) and supplies a relative_fd pointing to a netns link,

emm, the link referenced by relative_fd cannot be a non-netkit or 
non-tcx link. We need to add a restriction here.

> bpf_mprog_link() executes without an RCU read-side critical section.
> 
> Could this thread be preempted, allowing a concurrent BPF_LINK_UPDATE on
> the same netns link to replace and free link->prog? When the first thread
> resumes, it would dereference link->prog->type where link->prog now points
> to freed memory.
> 
>>   	if (prog) {
>>   		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
>> +		prog_id = prog->aux->id;
>> +	}
>> +	rcu_read_unlock();
>> +	rcu_read_unlock_trace();
> 

Re: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

> In bpf_link_show_fdinfo and bpf_link_get_info_by_fd, link->prog is
> accessed without holding any locks. If the prog is concurrently replaced
> via bpf_link_update, the old prog can be freed, leading to a potential
> UAF issue.
>
> Fix this by holding both normal RCU and tasks trace RCU read locks
> before dereferencing the prog, as BPF_LINK_TYPE_ITER support both
> non-sleepable and sleepable progs.
>
> Fixes: f9d041271cf4 ("bpf: Refactor bpf_link update handling")

Is the Fixes: tag pointing at the right commit?

f9d041271cf4 ("bpf: Refactor bpf_link update handling") only refactored
the existing update path into the bpf_link_ops->update_prog method.  It
does not look like it introduced the ability to concurrently replace and
free link->prog.

The unlocked reader in bpf_link_show_fdinfo() already existed by then,
and the racing replacement path (the BPF_LINK_UPDATE command with
xchg(&link->prog, new_prog) followed by bpf_prog_put(old_prog)) seems to
have first appeared in:

  0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")

Should the tag instead be:

  Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29715011327

Re: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[Pu Lehui]

On 2026/7/20 12:04, bot+bpf-ci@kernel.org wrote:
>> In bpf_link_show_fdinfo and bpf_link_get_info_by_fd, link->prog is
>> accessed without holding any locks. If the prog is concurrently replaced
>> via bpf_link_update, the old prog can be freed, leading to a potential
>> UAF issue.
>>
>> Fix this by holding both normal RCU and tasks trace RCU read locks
>> before dereferencing the prog, as BPF_LINK_TYPE_ITER support both
>> non-sleepable and sleepable progs.
>>
>> Fixes: f9d041271cf4 ("bpf: Refactor bpf_link update handling")
> 
> Is the Fixes: tag pointing at the right commit?
> 
> f9d041271cf4 ("bpf: Refactor bpf_link update handling") only refactored
> the existing update path into the bpf_link_ops->update_prog method.  It
> does not look like it introduced the ability to concurrently replace and
> free link->prog.
> 
> The unlocked reader in bpf_link_show_fdinfo() already existed by then,
> and the racing replacement path (the BPF_LINK_UPDATE command with
> xchg(&link->prog, new_prog) followed by bpf_prog_put(old_prog)) seems to
> have first appeared in:
> 
>    0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
> 
> Should the tag instead be:
> 
>    Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")

make sense.

> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29715011327

[PATCH bpf v3 2/3] bpf, cgroup: Fix storage not restored when update_effective_progs failed

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

In __cgroup_bpf_attach(), if update_effective_progs() fails, pl->storage
is not restored to old_storage. Since new_storage is subsequently freed,
pl->storage is left pointing to freed memory, leading to a potential UAF
issue.

Fix this by saving the old_storage before assigning the new one, and
restoring it in the error path.

Fixes: 7d9c3427894f ("bpf: Make cgroup storages shared between programs on the same cgroup")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 kernel/bpf/cgroup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 4355ccb78a9c..70757ae45934 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -813,6 +813,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 	struct bpf_prog *old_prog = NULL;
 	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+	struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_prog *new_prog = prog ? : link->link.prog;
 	enum cgroup_bpf_attach_type atype;
 	struct bpf_prog_list *pl;
@@ -883,6 +884,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 	pl->prog = prog;
 	pl->link = link;
 	pl->flags = flags;
+	bpf_cgroup_storages_assign(old_storage, pl->storage);
 	bpf_cgroup_storages_assign(pl->storage, storage);
 	cgrp->bpf.flags[atype] = saved_flags;
 
@@ -916,6 +918,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 		pl->prog = old_prog;
 		pl->link = NULL;
 	}
+	bpf_cgroup_storages_assign(pl->storage, old_storage);
 	bpf_cgroup_storages_free(new_storage);
 	if (!old_prog) {
 		hlist_del(&pl->node);
-- 
2.34.1

Re: [PATCH bpf v3 2/3] bpf, cgroup: Fix storage not restored when update_effective_progs failed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Uninitialized memory is read when saving `pl->storage` into `old_storage` for newly allocated `bpf_prog_list` entries.

Pre-existing issues:
- [High] If the first program attach to a cgroup fails, `cgrp->bpf.flags[atype]` is not rolled back, leading to a complete bypass of ancestor mandatory security policies.
--

--- Patch [2]: [PATCH bpf v3 2/3] bpf, cgroup: Fix storage not restored when update_effective_progs failed ---
commit 30470afb00630f3ed1a914958997d62e12b1f60a
Author: Pu Lehui <pulehui@huawei.com>

bpf, cgroup: Fix storage not restored when update_effective_progs failed

In __cgroup_bpf_attach(), if update_effective_progs() fails, pl->storage
is not restored to old_storage. This patch saves the old_storage before
assigning the new one, and restores it in the error path.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
[ ... ]
> @@ -883,6 +884,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
>  	pl->prog = prog;
>  	pl->link = link;
>  	pl->flags = flags;
> +	bpf_cgroup_storages_assign(old_storage, pl->storage);

[Severity: Medium]
When __cgroup_bpf_attach() allocates a new bpf_prog_list entry, it does so
without zeroing the memory:

        pl = kmalloc_obj(*pl);

Because pl is not zero-initialized, does calling bpf_cgroup_storages_assign()
here result in reading uninitialized garbage pointers from pl->storage and
copying them into old_storage? This could trigger KMSAN uninit-value warnings
when attaching a new program.

>  	bpf_cgroup_storages_assign(pl->storage, storage);
>  	cgrp->bpf.flags[atype] = saved_flags;
>  
[ ... ]
> @@ -916,6 +918,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
>  		pl->prog = old_prog;
>  		pl->link = NULL;
>  	}
> +	bpf_cgroup_storages_assign(pl->storage, old_storage);
>  	bpf_cgroup_storages_free(new_storage);
>  	if (!old_prog) {
>  		hlist_del(&pl->node);
>  		kfree(pl);
>  	}
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error path, if
the very first program attach to a cgroup fails (for example, if
update_effective_progs() fails), the newly added list entry is deleted and
freed here, making the list empty again.

However, the flags set earlier in the function are not rolled back:

        cgrp->bpf.flags[atype] = saved_flags;

Could an attacker attempt an attach with BPF_F_ALLOW_MULTI, intentionally
force an error (like hitting memory limits in update_effective_progs()), and
leave the compromised cgroup with permanently altered BPF flags? 

If the flags stay stuck, does this allow bypassing mandatory, non-overridable
programs from ancestor cgroups during compute_effective_progs()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720033055.1215477-1-pulehui@huaweicloud.com?part=2

Re: [PATCH bpf v3 2/3] bpf, cgroup: Fix storage not restored when update_effective_progs failed

[Pu Lehui]

On 2026/7/20 11:41, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Uninitialized memory is read when saving `pl->storage` into `old_storage` for newly allocated `bpf_prog_list` entries.
> 
> Pre-existing issues:
> - [High] If the first program attach to a cgroup fails, `cgrp->bpf.flags[atype]` is not rolled back, leading to a complete bypass of ancestor mandatory security policies.
> --
> 
> --- Patch [2]: [PATCH bpf v3 2/3] bpf, cgroup: Fix storage not restored when update_effective_progs failed ---
> commit 30470afb00630f3ed1a914958997d62e12b1f60a
> Author: Pu Lehui <pulehui@huawei.com>
> 
> bpf, cgroup: Fix storage not restored when update_effective_progs failed
> 
> In __cgroup_bpf_attach(), if update_effective_progs() fails, pl->storage
> is not restored to old_storage. This patch saves the old_storage before
> assigning the new one, and restores it in the error path.
> 
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
> [ ... ]
>> @@ -883,6 +884,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
>>   	pl->prog = prog;
>>   	pl->link = link;
>>   	pl->flags = flags;
>> +	bpf_cgroup_storages_assign(old_storage, pl->storage);
> 
> [Severity: Medium]
> When __cgroup_bpf_attach() allocates a new bpf_prog_list entry, it does so
> without zeroing the memory:
> 
>          pl = kmalloc_obj(*pl);
> 
> Because pl is not zero-initialized, does calling bpf_cgroup_storages_assign()
> here result in reading uninitialized garbage pointers from pl->storage and
> copying them into old_storage? This could trigger KMSAN uninit-value warnings
> when attaching a new program.

will fix in next

> 
>>   	bpf_cgroup_storages_assign(pl->storage, storage);
>>   	cgrp->bpf.flags[atype] = saved_flags;
>>   
> [ ... ]
>> @@ -916,6 +918,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
>>   		pl->prog = old_prog;
>>   		pl->link = NULL;
>>   	}
>> +	bpf_cgroup_storages_assign(pl->storage, old_storage);
>>   	bpf_cgroup_storages_free(new_storage);
>>   	if (!old_prog) {
>>   		hlist_del(&pl->node);
>>   		kfree(pl);
>>   	}
>>   	return err;
>>   }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at the error path, if
> the very first program attach to a cgroup fails (for example, if
> update_effective_progs() fails), the newly added list entry is deleted and
> freed here, making the list empty again.
> 
> However, the flags set earlier in the function are not rolled back:
> 
>          cgrp->bpf.flags[atype] = saved_flags;
> 
> Could an attacker attempt an attach with BPF_F_ALLOW_MULTI, intentionally
> force an error (like hitting memory limits in update_effective_progs()), and
> leave the compromised cgroup with permanently altered BPF flags?
> 
> If the flags stay stuck, does this allow bypassing mandatory, non-overridable
> programs from ancestor cgroups during compute_effective_progs()?
> 

It's hard to make update_effective_progs fail in __cgroup_bpf_attach, 
but for the sake of code rigor, I will include the storage and flags 
rollbacks in the original bugfix patch.

[PATCH bpf v3 3/3] bpf, cgroup: Fix storage null-ptr-deref after replacing prog

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

Syzkaller reported a storage null-ptr-deref issue after replacing prog.
This occurs in the following scenario:
1. prog A, an empty prog, is attached to a cgrp.
2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the
   bpf_get_local_storage helper.
3. link_update is called to replace prog A with prog B.

The reason is that __cgroup_bpf_replace fails to alloc and assign the
required cgrp storage for the incoming replacement prog. Consequently,
the new prog inherits an uninit storage, leading to null-ptr-deref panic
when kick the new prog.

Fix this by properly allocating the storage and comparing the old and
new storage pointers. If the storage changed, fallback to
update_effective_progs which performs a RCU-safe update of the entire
array. If the storage remains unchanged, we can safely retain the
fast-path in-place update.

Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 kernel/bpf/cgroup.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 70757ae45934..dad2d6a1d96d 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1035,11 +1035,17 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
 				struct bpf_cgroup_link *link,
 				struct bpf_prog *new_prog)
 {
+	struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+	struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+	enum bpf_cgroup_storage_type stype;
 	enum cgroup_bpf_attach_type atype;
+	bool storage_changed = false;
 	struct bpf_prog *old_prog;
 	struct bpf_prog_list *pl;
 	struct hlist_head *progs;
 	bool found = false;
+	int err;
 
 	atype = bpf_cgroup_atype_find(link->link.attach_type, new_prog->aux->attach_btf_id);
 	if (atype < 0)
@@ -1059,10 +1065,39 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
 	if (!found)
 		return -ENOENT;
 
+	if (bpf_cgroup_storages_alloc(storage, new_storage, link->link.attach_type,
+				      new_prog, cgrp))
+		return -ENOMEM;
+
+	for_each_cgroup_storage_type(stype) {
+		if (storage[stype] != pl->storage[stype]) {
+			storage_changed = true;
+			break;
+		}
+	}
+
 	cgrp->bpf.revisions[atype] += 1;
 	old_prog = xchg(&link->link.prog, new_prog);
-	replace_effective_prog(cgrp, atype, pl);
+
+	if (!storage_changed) {
+		replace_effective_prog(cgrp, atype, pl);
+		bpf_prog_put(old_prog);
+		return 0;
+	}
+
+	bpf_cgroup_storages_assign(old_storage, pl->storage);
+	bpf_cgroup_storages_assign(pl->storage, storage);
+	err = update_effective_progs(cgrp, atype);
+	if (err) {
+		xchg(&link->link.prog, old_prog);
+		bpf_cgroup_storages_assign(pl->storage, old_storage);
+		bpf_cgroup_storages_free(new_storage);
+		cgrp->bpf.revisions[atype] -= 1;
+		return err;
+	}
+
 	bpf_prog_put(old_prog);
+	bpf_cgroup_storages_link(new_storage, cgrp, link->link.attach_type);
 	return 0;
 }
 
-- 
2.34.1