On Sun, Nov 11, 2018 at 08:38:53PM +0000, Nadav Amit wrote: > From: Peter Zijlstra > > On Sat, Nov 10, 2018 at 03:17:27PM -0800, Nadav Amit wrote: > >> +void __init poking_init(void) > >> +{ > >> + spinlock_t *ptl; > >> + pte_t *ptep; > >> + > >> + poking_mm = copy_init_mm(); > >> + if (!poking_mm) { > >> + pr_err("x86/mm: error setting a separate poking address space"); > >> + return; > >> + } > >> + > >> + /* > >> + * Randomize the poking address, but make sure that the following page > >> + * will be mapped at the same PMD. We need 2 pages, so find space for 3, > >> + * and adjust the address if the PMD ends after the first one. > >> + */ > >> + poking_addr = TASK_UNMAPPED_BASE + > >> + (kaslr_get_random_long("Poking") & PAGE_MASK) % > >> + (TASK_SIZE - TASK_UNMAPPED_BASE - 3 * PAGE_SIZE); > >> + > >> + if (((poking_addr + PAGE_SIZE) & ~PMD_MASK) == 0) > >> + poking_addr += PAGE_SIZE; > >> + > >> + /* > >> + * We need to trigger the allocation of the page-tables that will be > >> + * needed for poking now. Later, poking may be performed in an atomic > >> + * section, which might cause allocation to fail. > >> + */ > >> + ptep = get_locked_pte(poking_mm, poking_addr, &ptl); > >> + if (!WARN_ON(!ptep)) > >> + pte_unmap_unlock(ptep, ptl); > >> +} > > > > The difference in how we deal with -ENOMEM here is weird. I think we > > have a _lot_ of code that simply hard assumes we don't fail memory alloc > > on init. > > > > I for instance would not mind to simply remove both branches and let the > > kernel crash and burn if we ever fail here. > > Actually, now that we removed the fallback of patching without poking_mm, a > failure to allocate poking_mm should have had a BUG_ON(). > > For the second case, I think we still need either WARN_ON() or BUG_ON(), at > least as some sort of an in-code comment. I’ll change it to BUG_ON() if you > prefer.
Sure, two BUG_ON()s works for me.