Re: [PATCH] powerpc: Fix PTE page address mismatch in pgtable ctor/dtor

2013-12-07 Thread Hong H. Pham

There is a typo in the v2 patch, please disregard it.  The v3 patch with
the correction will follow.

Thanks,
Hong

On 12/06/2013 11:15 AM, Hong H. Pham wrote:

Hi Aneesh,

On 12/06/2013 05:38 AM, Aneesh Kumar K.V wrote:


can you also specifiy the config details here. ie, 4K page size functions
are broken ?


My PPC64 config has SMP and 4K page size enabled.  I re-tested with 64K page 
size,
and the problem is not present.

I have added a note that this problem affects only PPC32 and PPC64 SMP kernels.
On PPC64, the problem is limited to 4K page size.


make it closer to what it was before,

pgtable_page_dtor(table);
pgtable_free_tlb(tlb, page_address(table), 0);

This is what we had before

-static inline void __pte_free_tlb(struct mmu_gather *tlb, struct page *ptepage,
- unsigned long address)
-{
-   tlb_flush_pgtable(tlb, address);
-   pgtable_page_dtor(ptepage);
-   pgtable_free_tlb(tlb, page_address(ptepage), 0);
-}


-aneesh



Done.

Thanks,
Hong

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: Fix PTE page address mismatch in pgtable ctor/dtor

2013-12-06 Thread Aneesh Kumar K.V
Hong H. Pham hong.p...@windriver.com writes:

 In pte_alloc_one(), pgtable_page_ctor() is passed an address that has
 not been converted by page_address() to the newly allocated PTE page.

 When the PTE is freed, __pte_free_tlb() calls pgtable_page_dtor()
 with an address to the PTE page that has been converted by page_address().
 The mismatch in the PTE's page address causes pgtable_page_dtor() to access
 invalid memory, so resources for that PTE (such as the page lock) is not
 properly cleaned up.

 This bug was introduced by commit d614bb041209fd7cb5e4b35e11a7b2f6ee8f62b8
 powerpc: Move the pte free routines from common header.

 On a preempt-rt kernel, a spinlock is dynamically allocated for each
 PTE in pgtable_page_ctor().  When the PTE is freed, calling
 pgtable_page_dtor() with a mismatched page address causes a memory leak,
 as the pointer to the PTE's spinlock is bogus.

 On mainline, there isn't any immediately obvious symptoms, but the
 problem still exists here.


can you also specifiy the config details here. ie, 4K page size functions
are broken ?


 Fixes: d614bb041209fd7c powerpc: Move the pte free routes from common header
 Cc: Paul Mackerras pau...@samba.org
 Cc: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
 Cc: linux-stable sta...@vger.kernel.org # v3.10+
 Signed-off-by: Hong H. Pham hong.p...@windriver.com
 ---
  arch/powerpc/include/asm/pgalloc-32.h | 2 +-
  arch/powerpc/include/asm/pgalloc-64.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/arch/powerpc/include/asm/pgalloc-32.h 
 b/arch/powerpc/include/asm/pgalloc-32.h
 index 27b2386..7ff24f0 100644
 --- a/arch/powerpc/include/asm/pgalloc-32.h
 +++ b/arch/powerpc/include/asm/pgalloc-32.h
 @@ -87,7 +87,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, 
 pgtable_t table,
   struct page *page = page_address(table);

   tlb_flush_pgtable(tlb, address);
 - pgtable_page_dtor(page);
 + pgtable_page_dtor(table);
   pgtable_free_tlb(tlb, page, 0);
  }
  #endif /* _ASM_POWERPC_PGALLOC_32_H */
 diff --git a/arch/powerpc/include/asm/pgalloc-64.h 
 b/arch/powerpc/include/asm/pgalloc-64.h
 index f65e27b..b187dc5 100644
 --- a/arch/powerpc/include/asm/pgalloc-64.h
 +++ b/arch/powerpc/include/asm/pgalloc-64.h
 @@ -147,7 +147,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, 
 pgtable_t table,
   struct page *page = page_address(table);


That one is also wrong right ? why not 


   tlb_flush_pgtable(tlb, address);
 - pgtable_page_dtor(page);
 + pgtable_page_dtor(table);
   pgtable_free_tlb(tlb, page, 0);
  }


make it closer to what it was before,

pgtable_page_dtor(table);
pgtable_free_tlb(tlb, page_address(table), 0);

This is what we had before

-static inline void __pte_free_tlb(struct mmu_gather *tlb, struct page *ptepage,
- unsigned long address)
-{
-   tlb_flush_pgtable(tlb, address);
-   pgtable_page_dtor(ptepage);
-   pgtable_free_tlb(tlb, page_address(ptepage), 0);
-}


-aneesh

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: Fix PTE page address mismatch in pgtable ctor/dtor

2013-12-06 Thread Hong H. Pham

Hi Aneesh,

On 12/06/2013 05:38 AM, Aneesh Kumar K.V wrote:


can you also specifiy the config details here. ie, 4K page size functions
are broken ?


My PPC64 config has SMP and 4K page size enabled.  I re-tested with 64K page 
size,
and the problem is not present.

I have added a note that this problem affects only PPC32 and PPC64 SMP kernels.
On PPC64, the problem is limited to 4K page size.


make it closer to what it was before,

pgtable_page_dtor(table);
pgtable_free_tlb(tlb, page_address(table), 0);

This is what we had before

-static inline void __pte_free_tlb(struct mmu_gather *tlb, struct page *ptepage,
- unsigned long address)
-{
-   tlb_flush_pgtable(tlb, address);
-   pgtable_page_dtor(ptepage);
-   pgtable_free_tlb(tlb, page_address(ptepage), 0);
-}


-aneesh



Done.

Thanks,
Hong
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc: Fix PTE page address mismatch in pgtable ctor/dtor

2013-12-05 Thread Hong H. Pham
In pte_alloc_one(), pgtable_page_ctor() is passed an address that has
not been converted by page_address() to the newly allocated PTE page.

When the PTE is freed, __pte_free_tlb() calls pgtable_page_dtor()
with an address to the PTE page that has been converted by page_address().
The mismatch in the PTE's page address causes pgtable_page_dtor() to access
invalid memory, so resources for that PTE (such as the page lock) is not
properly cleaned up.

This bug was introduced by commit d614bb041209fd7cb5e4b35e11a7b2f6ee8f62b8
powerpc: Move the pte free routines from common header.

On a preempt-rt kernel, a spinlock is dynamically allocated for each
PTE in pgtable_page_ctor().  When the PTE is freed, calling
pgtable_page_dtor() with a mismatched page address causes a memory leak,
as the pointer to the PTE's spinlock is bogus.

On mainline, there isn't any immediately obvious symptoms, but the
problem still exists here.

Fixes: d614bb041209fd7c powerpc: Move the pte free routes from common header
Cc: Paul Mackerras pau...@samba.org
Cc: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: linux-stable sta...@vger.kernel.org # v3.10+
Signed-off-by: Hong H. Pham hong.p...@windriver.com
---
 arch/powerpc/include/asm/pgalloc-32.h | 2 +-
 arch/powerpc/include/asm/pgalloc-64.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pgalloc-32.h 
b/arch/powerpc/include/asm/pgalloc-32.h
index 27b2386..7ff24f0 100644
--- a/arch/powerpc/include/asm/pgalloc-32.h
+++ b/arch/powerpc/include/asm/pgalloc-32.h
@@ -87,7 +87,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, 
pgtable_t table,
struct page *page = page_address(table);
 
tlb_flush_pgtable(tlb, address);
-   pgtable_page_dtor(page);
+   pgtable_page_dtor(table);
pgtable_free_tlb(tlb, page, 0);
 }
 #endif /* _ASM_POWERPC_PGALLOC_32_H */
diff --git a/arch/powerpc/include/asm/pgalloc-64.h 
b/arch/powerpc/include/asm/pgalloc-64.h
index f65e27b..b187dc5 100644
--- a/arch/powerpc/include/asm/pgalloc-64.h
+++ b/arch/powerpc/include/asm/pgalloc-64.h
@@ -147,7 +147,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, 
pgtable_t table,
struct page *page = page_address(table);
 
tlb_flush_pgtable(tlb, address);
-   pgtable_page_dtor(page);
+   pgtable_page_dtor(table);
pgtable_free_tlb(tlb, page, 0);
 }
 
-- 
1.8.3.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev