Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: query effective progs without cgroup_mutex #5062

Closed
wants to merge 5 commits into from
Closed

bpf: query effective progs without cgroup_mutex #5062

wants to merge 5 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: query effective progs without cgroup_mutex
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6e61c5f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 7b99f75
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ee9fd0a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: bdeeed3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 79b3604
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 4d585f4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

1 similar comment
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 4d585f4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: d84b1a6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 04cb845
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 90564f1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 108598c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 108598c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@fomichev
bpf: export bpf_prog_array_copy_core
170d0e7 
No functional changes.

It will be used later on to copy prog array into a temporary
kernel buffer. I'm also changing its return type to errno
to be consistent with the rest of the similar helpers.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@fomichev
rculist: add hlist_for_each_rcu
e1bbb02 
Same as __hlist_for_each_rcu but uses rcu_dereference_raw
to suppress the warning from the update path (which is enforced
by extra cond expression).

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@fomichev
bpf: refactor __cgroup_bpf_query
ab4a9f9 
No functional changes.

This patch is here to make it easier to review the next one.
We'll be copying buffer to the userspace from the temporary
kernel one, so rename prog_ids to user_prog_ids and add new
extra variable (p) to traverse it.

Also, instead of decrementing total_cnt, introduce new 'remaning'
variable to keep track of where we are. We'll need original
total_cnt in the next patch to know how many bytes to copy back.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 108598c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

@fomichev
bpf: query effective progs without cgroup_mutex
7c48551 
When querying bpf prog list, we don't really need to hold
cgroup_mutex. There is only one caller of cgroup_bpf_query
(cgroup_bpf_prog_query) and it does cgroup_get/put, so we're
safe WRT cgroup going way. However, we if we stop grabbing
cgroup_mutex, we risk racing with the prog attach/detach path
to the same cgroup, so here is how to work it around.

We have two case:
1. querying effective array
2. querying non-effective list

(1) is easy because it's already a RCU-managed array, so all we
need is to make a copy of that array (under rcu read lock)
into a temporary buffer and copy that temporary buffer back
to userspace.

(2) is more involved because we keep the list of progs and it's
not managed by RCU. However, it seems relatively simple to
convert that hlist to the RCU-managed one: convert the readers
to use hlist_xxx_rcu and replace kfree with kfree_rcu. One
other notable place is cgroup_bpf_release where we replace
hlist_for_each_entry_safe with hlist_for_each_entry_rcu. This
should be safe because hlist_del_rcu does not remove/poison
forward pointer of the list entry, so it's safe to remove
the elements while iterating (without specially flavored
for_each_safe wrapper).

For (2), we also need to take care of flags. I added a bunch
of READ_ONCE/WRITE_ONCE to annotate lockless access. And I
also moved flag update path to happen before adding prog
to the list to make sure readers observe correct flags.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 3879122
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
version: 1

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=746854
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: bpf: export bpf_prog_array_copy_core
Applying: rculist: add hlist_for_each_rcu
Applying: bpf: refactor __cgroup_bpf_query
Applying: bpf: query effective progs without cgroup_mutex
Using index info to reconstruct a base tree...
M	kernel/bpf/cgroup.c
Falling back to patching base and 3-way merge...
Auto-merging kernel/bpf/cgroup.c
CONFLICT (content): Merge conflict in kernel/bpf/cgroup.c
Patch failed at 0004 bpf: query effective progs without cgroup_mutex
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc kernel/bpf/cgroup.c
index 194c7767f815,981897ba8965..000000000000
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@@ -282,15 -282,17 +282,17 @@@ static void cgroup_bpf_release(struct w
  
  	unsigned int atype;
  
 -	mutex_lock(&cgroup_mutex);
 +	cgroup_lock();
  
  	for (atype = 0; atype < ARRAY_SIZE(cgrp->bpf.progs); atype++) {
- 		struct hlist_head *progs = &cgrp->bpf.progs[atype];
+ 		struct hlist_head *progs;
  		struct bpf_prog_list *pl;
- 		struct hlist_node *pltmp;
  
- 		hlist_for_each_entry_safe(pl, pltmp, progs, node) {
- 			hlist_del(&pl->node);
+ 		progs = rcu_dereference_protected(&cgrp->bpf.progs[atype],
+ 						  lockdep_is_held(&cgroup_mutex));
+ 
+ 		hlist_for_each_entry_rcu(pl, progs, node) {
+ 			hlist_del_rcu(&pl->node);
  			if (pl->prog) {
  				if (pl->prog->expected_attach_type == BPF_LSM_CGROUP)
  					bpf_trampoline_unlink_cgroup_shim(pl->prog);
@@@ -1116,17 -1140,17 +1140,23 @@@ static int cgroup_bpf_query(struct cgro
  		p += cnt;
  		remaining -= cnt;
  	}
- 	return ret;
- }
+ 	if (remaining != 0) {
+ 		ret = -EAGAIN; /* raced with the detach */
+ 		goto free_user_ids;
+ 	}
  
- static int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
- 			    union bpf_attr __user *uattr)
- {
- 	int ret;
+ 	if (copy_to_user(user_prog_ids, prog_ids, sizeof(u32) * total_cnt))
+ 		ret = -EFAULT;
+ 
+ free_user_ids:
+ 	kfree(prog_ids);
  
++<<<<<<< HEAD
 +	cgroup_lock();
 +	ret = __cgroup_bpf_query(cgrp, attr, uattr);
 +	cgroup_unlock();
++=======
++>>>>>>> bpf: query effective progs without cgroup_mutex
  	return ret;
  }

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot force-pushed the bpf-next_base branch 8 times, most recently from b48ebc5 to 878ccbd Compare May 17, 2023 18:54
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=746854 expired. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/746854=>bpf-next branch May 20, 2023 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next changes-requested V1
Projects
None yet
Milestone
No milestone
1 participant
@fomichev