* [PATCHv2] arm64: Correctly bounds check virt_addr_valid
@ 2016-09-21 22:25 ` Laura Abbott
0 siblings, 0 replies; 6+ messages in thread
From: Laura Abbott @ 2016-09-21 22:25 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Ard Biesheuvel, Mark Rutland
Cc: Laura Abbott, Kees Cook, linux-arm-kernel, linux-kernel
virt_addr_valid is supposed to return true if and only if virt_to_page
returns a valid page structure. The current macro does math on whatever
address is given and passes that to pfn_valid to verify. vmalloc and
module addresses can happen to generate a pfn that 'happens' to be
valid. Fix this by only performing the pfn_valid check on addresses that
have the potential to be valid.
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Properly parenthesize macro arguments. Re-factor to common macro.
Also in case it wasn't clear, there's no need to try and squeeze this
into 4.8. Hardened usercopy should have all the checks, this is just for
full correctness.
---
arch/arm64/include/asm/memory.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 31b7322..ba62df8 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -214,7 +214,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#ifndef CONFIG_SPARSEMEM_VMEMMAP
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
+#define _virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
#else
#define __virt_to_pgoff(kaddr) (((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
#define __page_to_voff(kaddr) (((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
@@ -222,11 +222,15 @@ static inline void *phys_to_virt(phys_addr_t x)
#define page_to_virt(page) ((void *)((__page_to_voff(page)) | PAGE_OFFSET))
#define virt_to_page(vaddr) ((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))
-#define virt_addr_valid(kaddr) pfn_valid((((u64)(kaddr) & ~PAGE_OFFSET) \
+#define _virt_addr_valid(kaddr) pfn_valid((((u64)(kaddr) & ~PAGE_OFFSET) \
+ PHYS_OFFSET) >> PAGE_SHIFT)
#endif
#endif
+#define _virt_addr_is_linear(kaddr) (((u64)(kaddr)) >= PAGE_OFFSET)
+#define virt_addr_valid(kaddr) (_virt_addr_is_linear(kaddr) && \
+ _virt_addr_valid(kaddr))
+
#include <asm-generic/memory_model.h>
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCHv2] arm64: Correctly bounds check virt_addr_valid
@ 2016-09-21 22:25 ` Laura Abbott
0 siblings, 0 replies; 6+ messages in thread
From: Laura Abbott @ 2016-09-21 22:25 UTC (permalink / raw)
To: linux-arm-kernel
virt_addr_valid is supposed to return true if and only if virt_to_page
returns a valid page structure. The current macro does math on whatever
address is given and passes that to pfn_valid to verify. vmalloc and
module addresses can happen to generate a pfn that 'happens' to be
valid. Fix this by only performing the pfn_valid check on addresses that
have the potential to be valid.
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Properly parenthesize macro arguments. Re-factor to common macro.
Also in case it wasn't clear, there's no need to try and squeeze this
into 4.8. Hardened usercopy should have all the checks, this is just for
full correctness.
---
arch/arm64/include/asm/memory.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 31b7322..ba62df8 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -214,7 +214,7 @@ static inline void *phys_to_virt(phys_addr_t x)
#ifndef CONFIG_SPARSEMEM_VMEMMAP
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
+#define _virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
#else
#define __virt_to_pgoff(kaddr) (((u64)(kaddr) & ~PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
#define __page_to_voff(kaddr) (((u64)(page) & ~VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
@@ -222,11 +222,15 @@ static inline void *phys_to_virt(phys_addr_t x)
#define page_to_virt(page) ((void *)((__page_to_voff(page)) | PAGE_OFFSET))
#define virt_to_page(vaddr) ((struct page *)((__virt_to_pgoff(vaddr)) | VMEMMAP_START))
-#define virt_addr_valid(kaddr) pfn_valid((((u64)(kaddr) & ~PAGE_OFFSET) \
+#define _virt_addr_valid(kaddr) pfn_valid((((u64)(kaddr) & ~PAGE_OFFSET) \
+ PHYS_OFFSET) >> PAGE_SHIFT)
#endif
#endif
+#define _virt_addr_is_linear(kaddr) (((u64)(kaddr)) >= PAGE_OFFSET)
+#define virt_addr_valid(kaddr) (_virt_addr_is_linear(kaddr) && \
+ _virt_addr_valid(kaddr))
+
#include <asm-generic/memory_model.h>
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2] arm64: Correctly bounds check virt_addr_valid
2016-09-21 22:25 ` Laura Abbott
@ 2016-09-21 23:49 ` Kees Cook
-1 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-09-21 23:49 UTC (permalink / raw)
To: Laura Abbott
Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Mark Rutland,
linux-arm-kernel, LKML
On Wed, Sep 21, 2016 at 3:25 PM, Laura Abbott <labbott@redhat.com> wrote:
>
> virt_addr_valid is supposed to return true if and only if virt_to_page
> returns a valid page structure. The current macro does math on whatever
> address is given and passes that to pfn_valid to verify. vmalloc and
> module addresses can happen to generate a pfn that 'happens' to be
> valid. Fix this by only performing the pfn_valid check on addresses that
> have the potential to be valid.
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Properly parenthesize macro arguments. Re-factor to common macro.
>
> Also in case it wasn't clear, there's no need to try and squeeze this
> into 4.8. Hardened usercopy should have all the checks, this is just for
> full correctness.
After this lands for 4.9, I should likely drop the checks that are in
hardened usercopy? That'll speed things up ever so slightly, and will
let us catch other architectures that have a weird
virt_addr_valid()...
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] arm64: Correctly bounds check virt_addr_valid
@ 2016-09-21 23:49 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-09-21 23:49 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 21, 2016 at 3:25 PM, Laura Abbott <labbott@redhat.com> wrote:
>
> virt_addr_valid is supposed to return true if and only if virt_to_page
> returns a valid page structure. The current macro does math on whatever
> address is given and passes that to pfn_valid to verify. vmalloc and
> module addresses can happen to generate a pfn that 'happens' to be
> valid. Fix this by only performing the pfn_valid check on addresses that
> have the potential to be valid.
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Properly parenthesize macro arguments. Re-factor to common macro.
>
> Also in case it wasn't clear, there's no need to try and squeeze this
> into 4.8. Hardened usercopy should have all the checks, this is just for
> full correctness.
After this lands for 4.9, I should likely drop the checks that are in
hardened usercopy? That'll speed things up ever so slightly, and will
let us catch other architectures that have a weird
virt_addr_valid()...
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] arm64: Correctly bounds check virt_addr_valid
2016-09-21 22:25 ` Laura Abbott
@ 2016-09-22 9:17 ` Will Deacon
-1 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2016-09-22 9:17 UTC (permalink / raw)
To: Laura Abbott
Cc: Catalin Marinas, Ard Biesheuvel, Mark Rutland, Kees Cook,
linux-arm-kernel, linux-kernel
On Wed, Sep 21, 2016 at 03:25:04PM -0700, Laura Abbott wrote:
>
> virt_addr_valid is supposed to return true if and only if virt_to_page
> returns a valid page structure. The current macro does math on whatever
> address is given and passes that to pfn_valid to verify. vmalloc and
> module addresses can happen to generate a pfn that 'happens' to be
> valid. Fix this by only performing the pfn_valid check on addresses that
> have the potential to be valid.
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Properly parenthesize macro arguments. Re-factor to common macro.
>
> Also in case it wasn't clear, there's no need to try and squeeze this
> into 4.8. Hardened usercopy should have all the checks, this is just for
> full correctness.
Thanks, I'll push this onto for-next/core later today.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] arm64: Correctly bounds check virt_addr_valid
@ 2016-09-22 9:17 ` Will Deacon
0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2016-09-22 9:17 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 21, 2016 at 03:25:04PM -0700, Laura Abbott wrote:
>
> virt_addr_valid is supposed to return true if and only if virt_to_page
> returns a valid page structure. The current macro does math on whatever
> address is given and passes that to pfn_valid to verify. vmalloc and
> module addresses can happen to generate a pfn that 'happens' to be
> valid. Fix this by only performing the pfn_valid check on addresses that
> have the potential to be valid.
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Properly parenthesize macro arguments. Re-factor to common macro.
>
> Also in case it wasn't clear, there's no need to try and squeeze this
> into 4.8. Hardened usercopy should have all the checks, this is just for
> full correctness.
Thanks, I'll push this onto for-next/core later today.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-22 9:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21 22:25 [PATCHv2] arm64: Correctly bounds check virt_addr_valid Laura Abbott
2016-09-21 22:25 ` Laura Abbott
2016-09-21 23:49 ` Kees Cook
2016-09-21 23:49 ` Kees Cook
2016-09-22 9:17 ` Will Deacon
2016-09-22 9:17 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.