Re: [PATCH bpf 2/2] bpf: fix OOB devmap writes when deleting elements

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> 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")
> Reported-by: Jordy Zomer <jordyzomer@google.com>
> Suggested-by: Jordy Zomer <jordyzomer@google.com>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>