Created attachment 197279 [details] Patch_for_RISCV_OUT-OF-BOUNDS-ACCESS There is a potential out-of-bounds access in function pmap_bootstrap (sys/riscv/riscv/pmap.c). 563 void 564 pmap_bootstrap(vm_offset_t l1pt, vm_paddr_t kernstart, vm_size_t kernlen) 565 { 566 u_int l1_slot, l2_slot, avail_slot, map_slot, used_map_slot; 567 uint64_t kern_delta; 568 pt_entry_t *l2; 569 vm_offset_t va, freemempos; 570 vm_offset_t dpcpu, msgbufpv; 571 vm_paddr_t pa, min_pa, max_pa; 572 int i; ... 621 map_slot = avail_slot = 0; 622 for (; map_slot < (physmap_idx * 2); map_slot += 2) { 623 if (physmap[map_slot] == physmap[map_slot + 1]) 624 continue; 625 626 if (physmap[map_slot] <= pa && 627 physmap[map_slot + 1] > pa) 628 break; 629 630 phys_avail[avail_slot] = physmap[map_slot]; 631 phys_avail[avail_slot + 1] = physmap[map_slot + 1]; 632 physmem += (phys_avail[avail_slot + 1] - 633 phys_avail[avail_slot]) >> PAGE_SHIFT; 634 avail_slot += 2; 635 } 636 637 /* Add the memory before the kernel */ 638 if (physmap[avail_slot] < pa) { 639 phys_avail[avail_slot] = physmap[map_slot]; 640 phys_avail[avail_slot + 1] = pa; 641 physmem += (phys_avail[avail_slot + 1] - 642 phys_avail[avail_slot]) >> PAGE_SHIFT; 643 avail_slot += 2; 644 } ... 737 } avail_slot may bigger or equal than PHYS_AVAIL_SIZE - 2 in loop (line 634). Then, there would be out-of-bounds access for phys_avail array in line 630, 631, 639, 640 and so on. The attachment is the proposal patch for this vulnerability.
ping
Created attachment 199144 [details] proposed patch I don't understand the need for the first hunk of the patch. In the for-loop, avail_slot is only used to index phys_avail[], which has size PHYSMAP_SIZE+2. In the second hunk, we should test avail_slot < PHYS_AVAIL_SIZE - 2 before using avail_slot as an index. However, I don't quite understand the code in the second hunk. It's checking whether the loop exited because it found a physmem range containing KERNBASE - kern_delta, so why is it using avail_slot as the index? I think the test is just wrong. The attached patch changes that code according to my understand of what it's supposed to be doing.
A commit references this bug: Author: markj Date: Fri Dec 14 18:50:32 UTC 2018 New revision: 342093 URL: https://svnweb.freebsd.org/changeset/base/342093 Log: Clean up the riscv pmap_bootstrap() implementation. - Build up phys_avail[] in a single loop, excluding memory used by the loaded kernel. - Fix an array indexing bug in the aforementioned phys_avail[] initialization.[1] - Remove some unneeded code copied from the arm64 implementation. PR: 231515 [1] Reviewed by: jhb MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D18464 Changes: head/sys/riscv/riscv/pmap.c
A commit references this bug: Author: markj Date: Wed Jan 2 16:40:55 UTC 2019 New revision: 342696 URL: https://svnweb.freebsd.org/changeset/base/342696 Log: MFC r342093: Clean up the riscv pmap_bootstrap() implementation. PR: 231515 Changes: _U stable/12/ stable/12/sys/riscv/riscv/pmap.c