lkml.org 
[lkml]   [2010]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] exofs: confusion between kmap() and kmap_atomic() api
On Sun, 09 May 2010 13:16:38 +0300
Boaz Harrosh <bharrosh@panasas.com> wrote:

> On 05/07/2010 12:05 PM, Dan Carpenter wrote:
> > For kmap_atomic() we call kunmap_atomic() on the returned pointer.
> > That's different from kmap() and kunmap() and so it's easy to get them
> > backwards.
> >
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> >
>
> Thank you Dan, I'll push it ASAP.

> Looks like a bad bug. So this is actually a leak, right? kunmap_atomic
> would detect the bad pointer and do nothing?

void kunmap_atomic(void *kvaddr, enum km_type type)
{
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
enum fixed_addresses idx = type + KM_TYPE_NR*smp_processor_id();
/*
* Force other mappings to Oops if they'll try to access this pte
* without first remap it. Keeping stale mappings around is a bad idea
* also, in case the page changes cacheability attributes or becomes
* a protected page in a hypervisor.
*/
if (vaddr == __fix_to_virt(FIX_KMAP_BEGIN+idx))
kpte_clear_flush(kmap_pte-idx, vaddr);
else {
#ifdef CONFIG_DEBUG_HIGHMEM
BUG_ON(vaddr < PAGE_OFFSET);
BUG_ON(vaddr >= (unsigned long)high_memory);
#endif
}
pagefault_enable();
}
if CONFIG_DEBUG_HIGHMEM=y, kunmap_atomic() will go BUG.

if CONFIG_DEBUG_HIGHMEM=n, kunmap_atomic() will do nothing, leaving the
pte pointing at the old page. Next time someone tries to use that
kmap_atomic() slot,

void *kmap_atomic_prot(struct page *page, enum km_type type, pgprot_t prot)
{
enum fixed_addresses idx;
unsigned long vaddr;
/* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
pagefault_disable();
if (!PageHighMem(page))
return page_address(page);
debug_kmap_atomic(type);

idx = type + KM_TYPE_NR*smp_processor_id();
vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
BUG_ON(!pte_none(*(kmap_pte-idx)));
set_pte(kmap_pte-idx, mk_pte(page, prot));

return (void *)vaddr;
}
kmap_atomic_prot() will go BUG because the pte wasn't cleared.


I can only assume that this code has never been run on i386. I'd suggest
adding a "Cc: <stable@kernel.org>" to the changelog if you have
expectations that anyone will try to run it on i386.



\
 
 \ /
  Last update: 2010-05-12 10:35    [from the cache]
©2003-2011 Jasper Spaans