Hi all,
This patch series includes backports for the changes that fix CVE-2023-52447.
Commit e6c86c513f44 ("rcu-tasks: Provide rcu_trace_implies_rcu_gp()") applied cleanly.
Commit 876673364161 ("bpf: Defer the free of inner map when necessary") had one significant conflict, which was due to missing commit 8d5a8011b35d ("bpf: Batch call_rcu callbacks instead of SLAB_TYPESAFE_BY_RCU."). The conflict was because of the switch to queue_work() from schedule_work() in __bpf_map_put(). From what I can tell, the switch to queue_work() from schedule_work() isn't relevant in the context of this bug, so I resolved the conflict by keeping schedule_work() and not including 8d5a8011b35d ("bpf: Batch call_rcu callbacks instead of SLAB_TYPESAFE_BY_RCU.").
I also noticed that commit a6fb03a9c9c8 ("bpf: add percpu stats for bpf_map elements insertions/deletions") is tagged as a stable dependency of commit 876673364161. However, I don't see the functions and fields added in that patch used at all in commit 876673364161. This patch was backported to linux-6.1.y, but a `git grep` seems to show that `bpf_map_init_elem_count` is never referenced in linux-6.1.y. It seems to me that this patch is not actually a dependency of commit 876673364161, so I didn't include it in this backport.
I ran the selftests added in commit 1624918be84a ("selftests/bpf: Add test cases for inner map"), and they passed with no KASAN warnings. However, I did not manage to find a kernel on which these tests did generate a KASAN warning, so the test result may not be very meaningful. Apart from that, my typical build+boot test passed.
Hou Tao (1): bpf: Defer the free of inner map when necessary
Paul E. McKenney (1): rcu-tasks: Provide rcu_trace_implies_rcu_gp()
include/linux/bpf.h | 7 ++++++- include/linux/rcupdate.h | 12 ++++++++++++ kernel/bpf/map_in_map.c | 11 ++++++++--- kernel/bpf/syscall.c | 26 ++++++++++++++++++++++++-- kernel/rcu/tasks.h | 2 ++ 5 files changed, 52 insertions(+), 6 deletions(-)
From: "Paul E. McKenney" paulmck@kernel.org
[ Upstream commit e6c86c513f440bec5f1046539c7e3c6c653842da ]
As an accident of implementation, an RCU Tasks Trace grace period also acts as an RCU grace period. However, this could change at any time. This commit therefore creates an rcu_trace_implies_rcu_gp() that currently returns true to codify this accident. Code relying on this accident must call this function to verify that this accident is still happening.
Reported-by: Hou Tao houtao@huaweicloud.com Signed-off-by: Paul E. McKenney paulmck@kernel.org Cc: Alexei Starovoitov ast@kernel.org Cc: Martin KaFai Lau martin.lau@linux.dev Link: https://lore.kernel.org/r/20221014113946.965131-2-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Stable-dep-of: 876673364161 ("bpf: Defer the free of inner map when necessary") Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 10108826191ab30388e8ae9d54505a628f78a7ec) Signed-off-by: Robert Kolchmeyer rkolchmeyer@google.com --- include/linux/rcupdate.h | 12 ++++++++++++ kernel/rcu/tasks.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 13bddb841ceb..e3b12de36e92 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -205,6 +205,18 @@ static inline void exit_tasks_rcu_stop(void) { } static inline void exit_tasks_rcu_finish(void) { } #endif /* #else #ifdef CONFIG_TASKS_RCU_GENERIC */
+/** + * rcu_trace_implies_rcu_gp - does an RCU Tasks Trace grace period imply an RCU grace period? + * + * As an accident of implementation, an RCU Tasks Trace grace period also + * acts as an RCU grace period. However, this could change at any time. + * Code relying on this accident must call this function to verify that + * this accident is still happening. + * + * You have been warned! + */ +static inline bool rcu_trace_implies_rcu_gp(void) { return true; } + /** * cond_resched_tasks_rcu_qs - Report potential quiescent states to RCU * diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index 28f628c70245..b24ef77325ee 100644 --- a/kernel/rcu/tasks.h +++ b/kernel/rcu/tasks.h @@ -1098,6 +1098,8 @@ static void rcu_tasks_trace_postscan(struct list_head *hop)
// Wait for late-stage exiting tasks to finish exiting. // These might have passed the call to exit_tasks_rcu_finish(). + + // If you remove the following line, update rcu_trace_implies_rcu_gp()!!! synchronize_rcu(); // Any tasks that exit after this point will set ->trc_reader_checked. }
From: Hou Tao houtao1@huawei.com
[ Upstream commit 876673364161da50eed6b472d746ef88242b2368 ]
When updating or deleting an inner map in map array or map htab, the map may still be accessed by non-sleepable program or sleepable program. However bpf_map_fd_put_ptr() decreases the ref-counter of the inner map directly through bpf_map_put(), if the ref-counter is the last one (which is true for most cases), the inner map will be freed by ops->map_free() in a kworker. But for now, most .map_free() callbacks don't use synchronize_rcu() or its variants to wait for the elapse of a RCU grace period, so after the invocation of ops->map_free completes, the bpf program which is accessing the inner map may incur use-after-free problem.
Fix the free of inner map by invoking bpf_map_free_deferred() after both one RCU grace period and one tasks trace RCU grace period if the inner map has been removed from the outer map before. The deferment is accomplished by using call_rcu() or call_rcu_tasks_trace() when releasing the last ref-counter of bpf map. The newly-added rcu_head field in bpf_map shares the same storage space with work field to reduce the size of bpf_map.
Fixes: bba1dc0b55ac ("bpf: Remove redundant synchronize_rcu.") Fixes: 638e4b825d52 ("bpf: Allows per-cpu maps and map-in-map in sleepable programs") Signed-off-by: Hou Tao houtao1@huawei.com Link: https://lore.kernel.org/r/20231204140425.1480317-5-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 62fca83303d608ad4fec3f7428c8685680bb01b0) Signed-off-by: Robert Kolchmeyer rkolchmeyer@google.com --- include/linux/bpf.h | 7 ++++++- kernel/bpf/map_in_map.c | 11 ++++++++--- kernel/bpf/syscall.c | 26 ++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 97d94bcba131..df15d4d445dd 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -192,9 +192,14 @@ struct bpf_map { */ atomic64_t refcnt ____cacheline_aligned; atomic64_t usercnt; - struct work_struct work; + /* rcu is used before freeing and work is only used during freeing */ + union { + struct work_struct work; + struct rcu_head rcu; + }; struct mutex freeze_mutex; atomic64_t writecnt; + bool free_after_mult_rcu_gp; };
static inline bool map_value_has_spin_lock(const struct bpf_map *map) diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c index af0f15db1bf9..4cf79f86bf45 100644 --- a/kernel/bpf/map_in_map.c +++ b/kernel/bpf/map_in_map.c @@ -110,10 +110,15 @@ void *bpf_map_fd_get_ptr(struct bpf_map *map,
void bpf_map_fd_put_ptr(struct bpf_map *map, void *ptr, bool need_defer) { - /* ptr->ops->map_free() has to go through one - * rcu grace period by itself. + struct bpf_map *inner_map = ptr; + + /* The inner map may still be used by both non-sleepable and sleepable + * bpf program, so free it after one RCU grace period and one tasks + * trace RCU grace period. */ - bpf_map_put(ptr); + if (need_defer) + WRITE_ONCE(inner_map->free_after_mult_rcu_gp, true); + bpf_map_put(inner_map); }
u32 bpf_map_fd_sys_lookup_elem(void *ptr) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 64206856a05c..d4b4a47081b5 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -487,6 +487,25 @@ static void bpf_map_put_uref(struct bpf_map *map) } }
+static void bpf_map_free_in_work(struct bpf_map *map) +{ + INIT_WORK(&map->work, bpf_map_free_deferred); + schedule_work(&map->work); +} + +static void bpf_map_free_rcu_gp(struct rcu_head *rcu) +{ + bpf_map_free_in_work(container_of(rcu, struct bpf_map, rcu)); +} + +static void bpf_map_free_mult_rcu_gp(struct rcu_head *rcu) +{ + if (rcu_trace_implies_rcu_gp()) + bpf_map_free_rcu_gp(rcu); + else + call_rcu(rcu, bpf_map_free_rcu_gp); +} + /* decrement map refcnt and schedule it for freeing via workqueue * (unrelying map implementation ops->map_free() might sleep) */ @@ -496,8 +515,11 @@ static void __bpf_map_put(struct bpf_map *map, bool do_idr_lock) /* bpf_map_free_id() must be called first */ bpf_map_free_id(map, do_idr_lock); btf_put(map->btf); - INIT_WORK(&map->work, bpf_map_free_deferred); - schedule_work(&map->work); + + if (READ_ONCE(map->free_after_mult_rcu_gp)) + call_rcu_tasks_trace(&map->rcu, bpf_map_free_mult_rcu_gp); + else + bpf_map_free_in_work(map); } }
linux-stable-mirror@lists.linaro.org
-
Robert Kolchmeyer -
Sasha Levin