lkml.org 
[lkml]   [2010]   [May]   [11]   [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 05/11/2010 02:03 AM, Andrew Morton wrote:
    > 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.
    >

    Right! Everyone I know runs 64bit. I will add the Cc: <stable@kernel.org>
    to the patch. Thanks.

    Boaz


    \
     
     \ /
      Last update: 2010-05-12 10:17    [W:0.028 / U:59.336 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site