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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <673ed3d8c25bb_157a20824@john.notmuch>
Date: Wed, 20 Nov 2024 22:31:52 -0800
From: John Fastabend <john.fastabend@...il.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>, 
 Maciej Fijalkowski <maciej.fijalkowski@...el.com>, 
 bpf@...r.kernel.org, 
 ast@...nel.org, 
 daniel@...earbox.net, 
 andrii@...nel.org
Cc: netdev@...r.kernel.org, 
 magnus.karlsson@...el.com, 
 bjorn@...nel.org, 
 maciej.fijalkowski@...el.com, 
 jordyzomer@...gle.com, 
 security@...nel.org
Subject: Re: [PATCH bpf 2/2] bpf: fix OOB devmap writes when deleting elements

Toke Høiland-Jørgensen wrote:
> Maciej Fijalkowski <maciej.fijalkowski@...el.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@...gle.com>
> > Suggested-by: Jordy Zomer <jordyzomer@...gle.com>
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>

Also think its worth sending this with cc stable.

Acked-by: John Fastabend <john.fastabend@...il.com>
> 
> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>
>