Thread (6 messages) 6 messages, 5 authors, 2021-07-28

Re: [PATCH] mm/migrate: fix page state accounting type conversion underflow

From: Nicholas Piggin <npiggin@gmail.com>
Date: 2021-07-26 01:43:39

Excerpts from David Hildenbrand's message of July 22, 2021 5:27 pm:
On 22.07.21 07:48, Nicholas Piggin wrote:
quoted
Similarly to commit 2da9f6305f306 ("mm/vmscan: fix NR_ISOLATED_FILE
corruption on 64-bit"), fix -ve int -> unsigned int -> long bug.

Reported-by: Alexey Kardashevskiy <redacted>
Fixes: c5fc5c3ae0c84 ("mm: migrate: account THP NUMA migration counters correctly")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
  mm/migrate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/migrate.c b/mm/migrate.c
index 34a9ad3e0a4f..7e240437e7d9 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2068,7 +2068,7 @@ int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma,
  	LIST_HEAD(migratepages);
  	new_page_t *new;
  	bool compound;
-	unsigned int nr_pages = thp_nr_pages(page);
+	int nr_pages = thp_nr_pages(page);
  
  	/*
  	 * PTE mapped THP or HugeTLB page can't reach here so the page could
This is too fragile to silently break again IMHO. Should we similarly to 
2da9f6305f306 handle the conversion explicitly inside the 
mod_node_page_state() call?
Casting to signed still has the fragility that the variable is unsigned
so negating it somewhere else would break. I was somewhat inconsistent 
in the fixes, but there is less code that uses the variable here so it's
simpler to change the type IMO.

Negating an unsigned type always gives you a non-negative number. 
Unfortunately types matter.

Thanks,
Nick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help