NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/57683: uvm_pglistalloc_s_ps() et al integer overflow



>Number:         57683
>Category:       kern
>Synopsis:       uvm_pglistalloc_s_ps() et al integer overflow
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 05 23:55:00 +0000 2023
>Originator:     Tobias Nygren
>Release:        10.99.10
>Organization:
>Environment:
aarch64
>Description:
When uvm_physseg_get_avail_end() is a large value (0x800000000 in my case),
some UVM functions can hit integer overflow when mapping device memory.
I see this when attempting to use nouveau on aarch64 (Ampere Altra),
hitting the assert here: https://github.com/NetBSD/src/blob/f0634c6a5ba141afed730885b76d7de8119e12c1/sys/uvm/uvm_pglist.c#L595

The attached patch fixes this problem and makes nouveau attach.

From inspection it looks like there are other functions that may have similar problems. At least uvm_pglistalloc_c_ps(), uvm_physseg_get_start_hint() and uvm_physseg_set_start_hint().
But problems will I think only occur here on systems with more than 16 TB of RAM.
>How-To-Repeat:

>Fix:
--- uvm_pglist.c	21 Dec 2021 08:27:49 -0000	1.90
+++ uvm_pglist.c	5 Nov 2023 23:44:19 -0000
@@ -523,7 +523,8 @@ static int
 uvm_pglistalloc_s_ps(uvm_physseg_t psi, int num, paddr_t low, paddr_t high,
     struct pglist *rlist)
 {
-	int todo, limit, candidate;
+	int todo;
+	long limit, candidate;
 	struct vm_page *pg;
 	bool second_pass;
 #ifdef PGALLOC_VERBOSE
@@ -546,9 +547,9 @@ uvm_pglistalloc_s_ps(uvm_physseg_t psi,
 		return -1;

 	todo = num;
-	candidate = uimax(low, uvm_physseg_get_avail_start(psi) +
+	candidate = ulmax(low, uvm_physseg_get_avail_start(psi) +
 	    uvm_physseg_get_start_hint(psi));
-	limit = uimin(high, uvm_physseg_get_avail_end(psi));
+	limit = ulmin(high, uvm_physseg_get_avail_end(psi));
 	pg = uvm_physseg_get_pg(psi, candidate - uvm_physseg_get_start(psi));
 	second_pass = false;

@@ -560,8 +561,8 @@ again:
 				break;
 			}
 			second_pass = true;
-			candidate = uimax(low, uvm_physseg_get_avail_start(psi));
-			limit = uimin(limit, uvm_physseg_get_avail_start(psi) +
+			candidate = ulmax(low, uvm_physseg_get_avail_start(psi));
+			limit = ulmin(limit, uvm_physseg_get_avail_start(psi) +
 			    uvm_physseg_get_start_hint(psi));
 			pg = uvm_physseg_get_pg(psi, candidate - uvm_physseg_get_start(psi));
 			goto again;
@@ -571,10 +572,10 @@ again:
 			paddr_t cidx = 0;
 			const uvm_physseg_t bank = uvm_physseg_find(candidate, &cidx);
 			KDASSERTMSG(bank == psi,
-			    "uvm_physseg_find(%#x) (%"PRIxPHYSSEG ") != psi %"PRIxPHYSSEG,
+			    "uvm_physseg_find(%#lx) (%"PRIxPHYSSEG ") != psi %"PRIxPHYSSEG,
 			     candidate, bank, psi);
 			KDASSERTMSG(cidx == candidate - uvm_physseg_get_start(psi),
-			    "uvm_physseg_find(%#x): %#"PRIxPADDR" != off %"PRIxPADDR,
+			    "uvm_physseg_find(%#lx): %#"PRIxPADDR" != off %"PRIxPADDR,
 			     candidate, cidx, candidate - uvm_physseg_get_start(psi));
 		}
 #endif
@@ -594,7 +595,7 @@ again:
 	uvm_physseg_set_start_hint(psi, candidate + 1 - uvm_physseg_get_avail_start(psi));
 	KASSERTMSG(uvm_physseg_get_start_hint(psi) <= uvm_physseg_get_avail_end(psi) -
 	    uvm_physseg_get_avail_start(psi),
-	    "%#x %u (%#x) <= %#"PRIxPADDR" - %#"PRIxPADDR" (%#"PRIxPADDR")",
+	    "%#lx %u (%#x) <= %#"PRIxPADDR" - %#"PRIxPADDR" (%#"PRIxPADDR")",
 	    candidate + 1,
 	    uvm_physseg_get_start_hint(psi),
 	    uvm_physseg_get_start_hint(psi),



Home | Main Index | Thread Index | Old Index