linux-kernel - Re: [PATCH v2 5/5] arm64: add KASan support

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Jul 2015 17:03:13 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Andrey Ryabinin <a.ryabinin@...sung.com>
Cc:	Arnd Bergmann <arnd@...db.de>,
	David Keitel <dkeitel@...eaurora.org>,
	Will Deacon <will.deacon@....com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Alexander Potapenko <glider@...gle.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 5/5] arm64: add KASan support

On Thu, Jul 16, 2015 at 06:30:11PM +0300, Andrey Ryabinin wrote:
> On 07/15/2015 07:37 PM, Catalin Marinas wrote:
> > Ok, so simply taking the call out of the loop won't work unless we
> > conditionally define these functions (wouldn't be too bad since we have
> > some #if CONFIG_PGTABLE_LEVELS already introduced by this patch but it
> > would be nicer without).
> > 
> > Anyway, I think we can keep the current iterations but exit early if
> > !pud_none() because it means we already populated it (reworked to match
> > other such patterns throughout the kernel with pgd_populate called from
> > the pud function; and untested):
> > 
> > void kasan_early_pmd_populate(pud_t *pud, unsigned long addr, unsigned long end)
> > {
> > 	pmd_t *pmd;
> > 	unsigned long next;
> > 
> > 	if (pud_none(*pud))
> > 		pud_populate(&init_mm, pud, kasan_zero_pmd);
> > 
> > 	pmd = pmd_offset(pud, addr);
> > 	do {
> > 		next = pmd_addr_end(addr, end);
> > 		kasan_early_pte_populate(pmd, addr, next);
> > 	} while (pmd++, addr = next, addr != end && pmd_none(*pmd));
> > }
> > 
> > void kasan_early_pud_populate(pgd_t *pgd, unsigned long addr, unsigned long end)
> > {
> > 	pud_t *pud;
> > 	unsigned long next;
> > 
> > 	if (pgd_none(*pgd))
> > 		pgd_populate(&init_mm, pgd, kasan_zero_pud);
> > 
> > 	pud = pud_offset(pgd, addr);
> > 	do {
> > 		next = pud_addr_end(addr, end);
> > 		kasan_early_pmd_populate(pud, addr, next);
> > 	} while (pud++, addr = next, addr != end && pud_none(*pud));
> > }
> > 
> > Given that we check pud_none() after the first iterations, it covers the
> > lower levels if needed.
> 
> I think this may work, if pud_none(*pud) will be replaced with !pud_val(*pud).
> We can't use pud_none() because with 2-level page tables it's always false, so
> we will never go down to pmd level where swapper_pg_dir populated.

The reason I used "do ... while" vs "while" or "for" is so that it gets
down to the pmd level. The iteration over pgd is always done in the top
loop via pgd_addr_end while the loops for missing levels (nopud, nopmd)
are always a single iteration whether we check for pud_none or not. But
when the level is present, we avoid looping when !pud_none().

> But you gave me another idea how we could use p?d_none() and avoid rewriting table entries:
> 
> 
> void kasan_early_pmd_populate(unsigned long start, unsigned long end, pte_t *pte)
> {
> 	unsigned long addr = start;
> 	long next;
> 
> 	do {
> 		pgd_t *pgd = pgd_offset_k(addr);
> 		pud_t *pud = pud_offset(pgd, addr);
> 		pmd_t *pmd = pmd_offset(pud, addr);
> 
> 		if (!pmd_none(*pmd))
> 			break;
> 
> 		pmd_populate_kernel(&init_mm, pmd, pte);
> 		next = pgd_addr_end(addr, end);
> 		next = pud_addr_end(addr, next)
> 		next = pmd_addr_end(addr, next);
> 	} while(addr = next, addr != end);
> }
> 
> void kasan_early_pud_populate(unsigned long start, unsigned long end, pmd_t *pmd)
> {
> 	unsigned long addr = start;
> 	long next;
> 
> 	do {
> 		pgd_t *pgd = pgd_offset_k(addr);
> 		pud_t *pud = pud_offset(pgd, addr);
> 
> 		if (!pud_none(*pud))
> 			break;
> 
> 		pud_populate(&init_mm, pud, pmd);
> 		next = pud_addr_end(addr, pgd_addr_end(addr, end));
> 	} while(addr = next, addr != end);
> }
> 
> 
> void kasan_early_pgd_populate(...)
> {
> 	//something similar to above
> 	....
> }
> 
> static void __init kasan_map_early_shadow(void)
> {
> 	kasan_early_pgd_populate(KASAN_SHADOW_START, KASAN_SHADOW_END, kasan_zero_pud);
> 	kasan_early_pud_populate(KASAN_SHADOW_START, KASAN_SHADOW_END, kasan_zero_pmd);
> 	kasan_early_pmd_populate(KASAN_SHADOW_START, KASAN_SHADOW_END, kasan_zero_pte);
> 	kasan_early_pte_populate();
> }

While this would probably work, you still need #ifdef's since
kasan_zero_pud is not defined with 2 and 3 levels. That's what I
initially thought we should do but since you didn't like the #ifdef's, I
came up with another proposal.

So, I still prefer my suggestion above unless you find a problem with
it.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/