From: Sasha Levin Date: Wed, 11 Dec 2024 18:37:05 +0000 (-0500) Subject: Fixes for 5.4 X-Git-Tag: v5.4.287~65 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=baeadad2457c79e5e0c41a29d7ee4852ab46a790;p=thirdparty%2Fkernel%2Fstable-queue.git Fixes for 5.4 Signed-off-by: Sasha Levin --- diff --git a/queue-5.4/bpf-fix-oob-devmap-writes-when-deleting-elements.patch b/queue-5.4/bpf-fix-oob-devmap-writes-when-deleting-elements.patch new file mode 100644 index 00000000000..26230d4a532 --- /dev/null +++ b/queue-5.4/bpf-fix-oob-devmap-writes-when-deleting-elements.patch @@ -0,0 +1,115 @@ +From cc06cd98141981686c79e55e45ecee177a085d4b Mon Sep 17 00:00:00 2001 +From: Sasha Levin +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 + +[ 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] +[ 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] + +Fixes: 546ac1ffb70d ("bpf: add devmap, a map for storing net device references") +CC: stable@vger.kernel.org +Reported-by: Jordy Zomer +Suggested-by: Jordy Zomer +Reviewed-by: Toke Høiland-Jørgensen +Acked-by: John Fastabend +Signed-off-by: Maciej Fijalkowski +Link: https://lore.kernel.org/r/20241122121030.716788-3-maciej.fijalkowski@intel.com +Signed-off-by: Alexei Starovoitov +Signed-off-by: Sasha Levin +--- + 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 + diff --git a/queue-5.4/series b/queue-5.4/series index 81ef4e7d694..155d18ef059 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -306,3 +306,5 @@ i3c-use-i3cdev-desc-info-instead-of-calling-i3c_devi.patch usb-chipidea-udc-handle-usb-error-interrupt-if-ioc-n.patch powerpc-prom_init-fixup-missing-powermac-size-cells.patch misc-eeprom-eeprom_93cx6-add-quirk-for-extra-read-cl.patch +xdp-simplify-devmap-cleanup.patch +bpf-fix-oob-devmap-writes-when-deleting-elements.patch diff --git a/queue-5.4/xdp-simplify-devmap-cleanup.patch b/queue-5.4/xdp-simplify-devmap-cleanup.patch new file mode 100644 index 00000000000..3527d2665a4 --- /dev/null +++ b/queue-5.4/xdp-simplify-devmap-cleanup.patch @@ -0,0 +1,142 @@ +From 189e44fecab2dc466f7a0d8414692fc2704f1008 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +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 + +[ 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 +Signed-off-by: Alexei Starovoitov +Acked-by: Toke Høiland-Jørgensen +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 +--- + 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 +