--- /dev/null
+From cc06cd98141981686c79e55e45ecee177a085d4b Mon Sep 17 00:00:00 2001
+From: Sasha Levin <sashal@kernel.org>
+Date: Fri, 22 Nov 2024 13:10:30 +0100
+Subject: bpf: fix OOB devmap writes when deleting elements
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
+
+[ Upstream commit ab244dd7cf4c291f82faacdc50b45cc0f55b674d ]
+
+Jordy reported issue against XSKMAP which also applies to DEVMAP - the
+index used for accessing map entry, due to being a signed integer,
+causes the OOB writes. Fix is simple as changing the type from int to
+u32, however, when compared to XSKMAP case, one more thing needs to be
+addressed.
+
+When map is released from system via dev_map_free(), we iterate through
+all of the entries and an iterator variable is also an int, which
+implies OOB accesses. Again, change it to be u32.
+
+Example splat below:
+
+[ 160.724676] BUG: unable to handle page fault for address: ffffc8fc2c001000
+[ 160.731662] #PF: supervisor read access in kernel mode
+[ 160.736876] #PF: error_code(0x0000) - not-present page
+[ 160.742095] PGD 0 P4D 0
+[ 160.744678] Oops: Oops: 0000 [#1] PREEMPT SMP
+[ 160.749106] CPU: 1 UID: 0 PID: 520 Comm: kworker/u145:12 Not tainted 6.12.0-rc1+ #487
+[ 160.757050] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.02.01.0008.031920191559 03/19/2019
+[ 160.767642] Workqueue: events_unbound bpf_map_free_deferred
+[ 160.773308] RIP: 0010:dev_map_free+0x77/0x170
+[ 160.777735] Code: 00 e8 fd 91 ed ff e8 b8 73 ed ff 41 83 7d 18 19 74 6e 41 8b 45 24 49 8b bd f8 00 00 00 31 db 85 c0 74 48 48 63 c3 48 8d 04 c7 <48> 8b 28 48 85 ed 74 30 48 8b 7d 18 48 85 ff 74 05 e8 b3 52 fa ff
+[ 160.796777] RSP: 0018:ffffc9000ee1fe38 EFLAGS: 00010202
+[ 160.802086] RAX: ffffc8fc2c001000 RBX: 0000000080000000 RCX: 0000000000000024
+[ 160.809331] RDX: 0000000000000000 RSI: 0000000000000024 RDI: ffffc9002c001000
+[ 160.816576] RBP: 0000000000000000 R08: 0000000000000023 R09: 0000000000000001
+[ 160.823823] R10: 0000000000000001 R11: 00000000000ee6b2 R12: dead000000000122
+[ 160.831066] R13: ffff88810c928e00 R14: ffff8881002df405 R15: 0000000000000000
+[ 160.838310] FS: 0000000000000000(0000) GS:ffff8897e0c40000(0000) knlGS:0000000000000000
+[ 160.846528] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
+[ 160.852357] CR2: ffffc8fc2c001000 CR3: 0000000005c32006 CR4: 00000000007726f0
+[ 160.859604] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
+[ 160.866847] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
+[ 160.874092] PKRU: 55555554
+[ 160.876847] Call Trace:
+[ 160.879338] <TASK>
+[ 160.881477] ? __die+0x20/0x60
+[ 160.884586] ? page_fault_oops+0x15a/0x450
+[ 160.888746] ? search_extable+0x22/0x30
+[ 160.892647] ? search_bpf_extables+0x5f/0x80
+[ 160.896988] ? exc_page_fault+0xa9/0x140
+[ 160.900973] ? asm_exc_page_fault+0x22/0x30
+[ 160.905232] ? dev_map_free+0x77/0x170
+[ 160.909043] ? dev_map_free+0x58/0x170
+[ 160.912857] bpf_map_free_deferred+0x51/0x90
+[ 160.917196] process_one_work+0x142/0x370
+[ 160.921272] worker_thread+0x29e/0x3b0
+[ 160.925082] ? rescuer_thread+0x4b0/0x4b0
+[ 160.929157] kthread+0xd4/0x110
+[ 160.932355] ? kthread_park+0x80/0x80
+[ 160.936079] ret_from_fork+0x2d/0x50
+[ 160.943396] ? kthread_park+0x80/0x80
+[ 160.950803] ret_from_fork_asm+0x11/0x20
+[ 160.958482] </TASK>
+
+Fixes: 546ac1ffb70d ("bpf: add devmap, a map for storing net device references")
+CC: stable@vger.kernel.org
+Reported-by: Jordy Zomer <jordyzomer@google.com>
+Suggested-by: Jordy Zomer <jordyzomer@google.com>
+Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
+Acked-by: John Fastabend <john.fastabend@gmail.com>
+Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
+Link: https://lore.kernel.org/r/20241122121030.716788-3-maciej.fijalkowski@intel.com
+Signed-off-by: Alexei Starovoitov <ast@kernel.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ kernel/bpf/devmap.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
+index e63ecb21e0554..40d4247378592 100644
+--- a/kernel/bpf/devmap.c
++++ b/kernel/bpf/devmap.c
+@@ -206,7 +206,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
+ static void dev_map_free(struct bpf_map *map)
+ {
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+- int i;
++ u32 i;
+
+ /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+ * so the programs (can be more than one that used this map) were
+@@ -512,7 +512,7 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
+ {
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *old_dev;
+- int k = *(u32 *)key;
++ u32 k = *(u32 *)key;
+
+ if (k >= map->max_entries)
+ return -EINVAL;
+@@ -535,7 +535,7 @@ static int dev_map_hash_delete_elem(struct bpf_map *map, void *key)
+ {
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *old_dev;
+- int k = *(u32 *)key;
++ u32 k = *(u32 *)key;
+ unsigned long flags;
+ int ret = -ENOENT;
+
+--
+2.43.0
+
--- /dev/null
+From 189e44fecab2dc466f7a0d8414692fc2704f1008 Mon Sep 17 00:00:00 2001
+From: Sasha Levin <sashal@kernel.org>
+Date: Thu, 19 Dec 2019 07:09:59 +0100
+Subject: xdp: Simplify devmap cleanup
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+From: Björn Töpel <bjorn.topel@intel.com>
+
+[ Upstream commit 0536b85239b8440735cdd910aae0eb076ebbb439 ]
+
+After the RCU flavor consolidation [1], call_rcu() and
+synchronize_rcu() waits for preempt-disable regions (NAPI) in addition
+to the read-side critical sections. As a result of this, the cleanup
+code in devmap can be simplified
+
+* There is no longer a need to flush in __dev_map_entry_free, since we
+ know that this has been done when the call_rcu() callback is
+ triggered.
+
+* When freeing the map, there is no need to explicitly wait for a
+ flush. It's guaranteed to be done after the synchronize_rcu() call
+ in dev_map_free(). The rcu_barrier() is still needed, so that the
+ map is not freed prior the elements.
+
+[1] https://lwn.net/Articles/777036/
+
+Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
+Signed-off-by: Alexei Starovoitov <ast@kernel.org>
+Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
+Link: https://lore.kernel.org/bpf/20191219061006.21980-2-bjorn.topel@gmail.com
+Stable-dep-of: ab244dd7cf4c ("bpf: fix OOB devmap writes when deleting elements")
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ kernel/bpf/devmap.c | 43 +++++--------------------------------------
+ 1 file changed, 5 insertions(+), 38 deletions(-)
+
+diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
+index 2370fc31169f5..e63ecb21e0554 100644
+--- a/kernel/bpf/devmap.c
++++ b/kernel/bpf/devmap.c
+@@ -206,7 +206,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
+ static void dev_map_free(struct bpf_map *map)
+ {
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+- int i, cpu;
++ int i;
+
+ /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+ * so the programs (can be more than one that used this map) were
+@@ -226,18 +226,6 @@ static void dev_map_free(struct bpf_map *map)
+ /* Make sure prior __dev_map_entry_free() have completed. */
+ rcu_barrier();
+
+- /* To ensure all pending flush operations have completed wait for flush
+- * list to empty on _all_ cpus.
+- * Because the above synchronize_rcu() ensures the map is disconnected
+- * from the program we can assume no new items will be added.
+- */
+- for_each_online_cpu(cpu) {
+- struct list_head *flush_list = per_cpu_ptr(dtab->flush_list, cpu);
+-
+- while (!list_empty(flush_list))
+- cond_resched();
+- }
+-
+ if (dtab->map.map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
+ for (i = 0; i < dtab->n_buckets; i++) {
+ struct bpf_dtab_netdev *dev;
+@@ -351,8 +339,7 @@ static int dev_map_hash_get_next_key(struct bpf_map *map, void *key,
+ return -ENOENT;
+ }
+
+-static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags,
+- bool in_napi_ctx)
++static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags)
+ {
+ struct bpf_dtab_netdev *obj = bq->obj;
+ struct net_device *dev = obj->dev;
+@@ -390,11 +377,7 @@ static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags,
+ for (i = 0; i < bq->count; i++) {
+ struct xdp_frame *xdpf = bq->q[i];
+
+- /* RX path under NAPI protection, can return frames faster */
+- if (likely(in_napi_ctx))
+- xdp_return_frame_rx_napi(xdpf);
+- else
+- xdp_return_frame(xdpf);
++ xdp_return_frame_rx_napi(xdpf);
+ drops++;
+ }
+ goto out;
+@@ -415,7 +398,7 @@ void __dev_map_flush(struct bpf_map *map)
+
+ rcu_read_lock();
+ list_for_each_entry_safe(bq, tmp, flush_list, flush_node)
+- bq_xmit_all(bq, XDP_XMIT_FLUSH, true);
++ bq_xmit_all(bq, XDP_XMIT_FLUSH);
+ rcu_read_unlock();
+ }
+
+@@ -446,7 +429,7 @@ static int bq_enqueue(struct bpf_dtab_netdev *obj, struct xdp_frame *xdpf,
+ struct xdp_bulk_queue *bq = this_cpu_ptr(obj->bulkq);
+
+ if (unlikely(bq->count == DEV_MAP_BULK_SIZE))
+- bq_xmit_all(bq, 0, true);
++ bq_xmit_all(bq, 0);
+
+ /* Ingress dev_rx will be the same for all xdp_frame's in
+ * bulk_queue, because bq stored per-CPU and must be flushed
+@@ -515,27 +498,11 @@ static void *dev_map_hash_lookup_elem(struct bpf_map *map, void *key)
+ return dev ? &dev->ifindex : NULL;
+ }
+
+-static void dev_map_flush_old(struct bpf_dtab_netdev *dev)
+-{
+- if (dev->dev->netdev_ops->ndo_xdp_xmit) {
+- struct xdp_bulk_queue *bq;
+- int cpu;
+-
+- rcu_read_lock();
+- for_each_online_cpu(cpu) {
+- bq = per_cpu_ptr(dev->bulkq, cpu);
+- bq_xmit_all(bq, XDP_XMIT_FLUSH, false);
+- }
+- rcu_read_unlock();
+- }
+-}
+-
+ static void __dev_map_entry_free(struct rcu_head *rcu)
+ {
+ struct bpf_dtab_netdev *dev;
+
+ dev = container_of(rcu, struct bpf_dtab_netdev, rcu);
+- dev_map_flush_old(dev);
+ free_percpu(dev->bulkq);
+ dev_put(dev->dev);
+ kfree(dev);
+--
+2.43.0
+
projects / thirdparty / kernel / stable-queue.git / commitdiff
