linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
@ 2017-06-09  8:22 Ard Biesheuvel
  2017-06-09  9:22 ` Mark Rutland
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2017-06-09  8:22 UTC (permalink / raw)
  To: linux-arm-kernel

Existing code that uses vmalloc_to_page() may assume that any
address for which is_vmalloc_addr() returns true may be passed
into vmalloc_to_page() to retrieve the associated struct page.

This is not un unreasonable assumption to make, but on architectures
that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
need to ensure that vmalloc_to_page() does not go off into the weeds
trying to dereference huge PUDs or PMDs as table entries.

Given that vmalloc() and vmap() themselves never create huge
mappings or deal with compound pages at all, there is no correct
answer in this case, so return NULL instead, and issue a warning.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v5: - fix typo

v4: - use pud_bad/pmd_bad instead of pud_huge/pmd_huge, which don't require
      changes to hugetlb.h, and give us what we need on all architectures
    - move WARN_ON_ONCE() calls out of conditionals
    - add explanatory comment

 mm/vmalloc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 34a1c3e46ed7..0fcd371266a4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -287,10 +287,21 @@ struct page *vmalloc_to_page(const void *vmalloc_addr)
 	if (p4d_none(*p4d))
 		return NULL;
 	pud = pud_offset(p4d, addr);
-	if (pud_none(*pud))
+
+	/*
+	 * Don't dereference bad PUD or PMD (below) entries. This will also
+	 * identify huge mappings, which we may encounter on architectures
+	 * that define CONFIG_HAVE_ARCH_HUGE_VMAP=y. Such regions will be
+	 * identified as vmalloc addresses by is_vmalloc_addr(), but are
+	 * not [unambiguously] associated with a struct page, so there is
+	 * no correct value to return for them.
+	 */
+	WARN_ON_ONCE(pud_bad(*pud));
+	if (pud_none(*pud) || pud_bad(*pud))
 		return NULL;
 	pmd = pmd_offset(pud, addr);
-	if (pmd_none(*pmd))
+	WARN_ON_ONCE(pmd_bad(*pmd));
+	if (pmd_none(*pmd) || pmd_bad(*pmd))
 		return NULL;
 
 	ptep = pte_offset_map(pmd, addr);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-09  8:22 [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings Ard Biesheuvel
@ 2017-06-09  9:22 ` Mark Rutland
  2017-06-09  9:27   ` Ard Biesheuvel
  2017-06-09 18:13 ` Laura Abbott
  2017-06-15 21:24 ` Andrew Morton
  2 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2017-06-09  9:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 09, 2017 at 08:22:26AM +0000, Ard Biesheuvel wrote:
> Existing code that uses vmalloc_to_page() may assume that any
> address for which is_vmalloc_addr() returns true may be passed
> into vmalloc_to_page() to retrieve the associated struct page.
> 
> This is not un unreasonable assumption to make, but on architectures
> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
> need to ensure that vmalloc_to_page() does not go off into the weeds
> trying to dereference huge PUDs or PMDs as table entries.
> 
> Given that vmalloc() and vmap() themselves never create huge
> mappings or deal with compound pages at all, there is no correct
> answer in this case, so return NULL instead, and issue a warning.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v5: - fix typo
> 
> v4: - use pud_bad/pmd_bad instead of pud_huge/pmd_huge, which don't require
>       changes to hugetlb.h, and give us what we need on all architectures
>     - move WARN_ON_ONCE() calls out of conditionals
>     - add explanatory comment
> 
>  mm/vmalloc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 34a1c3e46ed7..0fcd371266a4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -287,10 +287,21 @@ struct page *vmalloc_to_page(const void *vmalloc_addr)
>  	if (p4d_none(*p4d))
>  		return NULL;
>  	pud = pud_offset(p4d, addr);
> -	if (pud_none(*pud))
> +
> +	/*
> +	 * Don't dereference bad PUD or PMD (below) entries. This will also
> +	 * identify huge mappings, which we may encounter on architectures
> +	 * that define CONFIG_HAVE_ARCH_HUGE_VMAP=y. Such regions will be
> +	 * identified as vmalloc addresses by is_vmalloc_addr(), but are
> +	 * not [unambiguously] associated with a struct page, so there is
> +	 * no correct value to return for them.
> +	 */
> +	WARN_ON_ONCE(pud_bad(*pud));
> +	if (pud_none(*pud) || pud_bad(*pud))
>  		return NULL;

Nit: the WARN_ON_ONCE() can be folded into the conditional:

	if (pud_none(*pud) || WARN_ON_ONCE(pud_bad(*pud)))
		reutrn NULL;

>  	pmd = pmd_offset(pud, addr);
> -	if (pmd_none(*pmd))
> +	WARN_ON_ONCE(pmd_bad(*pmd));
> +	if (pmd_none(*pmd) || pmd_bad(*pmd))
>  		return NULL;

Likewise here.

Otherwise, looks good to me. FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

>  
>  	ptep = pte_offset_map(pmd, addr);
> -- 
> 2.9.3
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-09  9:22 ` Mark Rutland
@ 2017-06-09  9:27   ` Ard Biesheuvel
  2017-06-09  9:29     ` Mark Rutland
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-06-09  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 9 June 2017 at 09:22, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jun 09, 2017 at 08:22:26AM +0000, Ard Biesheuvel wrote:
>> Existing code that uses vmalloc_to_page() may assume that any
>> address for which is_vmalloc_addr() returns true may be passed
>> into vmalloc_to_page() to retrieve the associated struct page.
>>
>> This is not un unreasonable assumption to make, but on architectures
>> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
>> need to ensure that vmalloc_to_page() does not go off into the weeds
>> trying to dereference huge PUDs or PMDs as table entries.
>>
>> Given that vmalloc() and vmap() themselves never create huge
>> mappings or deal with compound pages at all, there is no correct
>> answer in this case, so return NULL instead, and issue a warning.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v5: - fix typo
>>
>> v4: - use pud_bad/pmd_bad instead of pud_huge/pmd_huge, which don't require
>>       changes to hugetlb.h, and give us what we need on all architectures
>>     - move WARN_ON_ONCE() calls out of conditionals

^^^

>>     - add explanatory comment
>>
>>  mm/vmalloc.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index 34a1c3e46ed7..0fcd371266a4 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -287,10 +287,21 @@ struct page *vmalloc_to_page(const void *vmalloc_addr)
>>       if (p4d_none(*p4d))
>>               return NULL;
>>       pud = pud_offset(p4d, addr);
>> -     if (pud_none(*pud))
>> +
>> +     /*
>> +      * Don't dereference bad PUD or PMD (below) entries. This will also
>> +      * identify huge mappings, which we may encounter on architectures
>> +      * that define CONFIG_HAVE_ARCH_HUGE_VMAP=y. Such regions will be
>> +      * identified as vmalloc addresses by is_vmalloc_addr(), but are
>> +      * not [unambiguously] associated with a struct page, so there is
>> +      * no correct value to return for them.
>> +      */
>> +     WARN_ON_ONCE(pud_bad(*pud));
>> +     if (pud_none(*pud) || pud_bad(*pud))
>>               return NULL;
>
> Nit: the WARN_ON_ONCE() can be folded into the conditional:
>
>         if (pud_none(*pud) || WARN_ON_ONCE(pud_bad(*pud)))
>                 reutrn NULL;
>
>>       pmd = pmd_offset(pud, addr);
>> -     if (pmd_none(*pmd))
>> +     WARN_ON_ONCE(pmd_bad(*pmd));
>> +     if (pmd_none(*pmd) || pmd_bad(*pmd))
>>               return NULL;
>
> Likewise here.
>

Actually, it was Dave who requested them to be taken out of the conditional.

> Otherwise, looks good to me. FWIW:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-09  9:27   ` Ard Biesheuvel
@ 2017-06-09  9:29     ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2017-06-09  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 09, 2017 at 09:27:15AM +0000, Ard Biesheuvel wrote:
> On 9 June 2017 at 09:22, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Jun 09, 2017 at 08:22:26AM +0000, Ard Biesheuvel wrote:
> >> v4: - use pud_bad/pmd_bad instead of pud_huge/pmd_huge, which don't require
> >>       changes to hugetlb.h, and give us what we need on all architectures
> >>     - move WARN_ON_ONCE() calls out of conditionals
> 
> ^^^

Ah, sorry. Clearly I scanned this too quickly.

> >> +     WARN_ON_ONCE(pud_bad(*pud));
> >> +     if (pud_none(*pud) || pud_bad(*pud))
> >>               return NULL;
> >
> > Nit: the WARN_ON_ONCE() can be folded into the conditional:
> >
> >         if (pud_none(*pud) || WARN_ON_ONCE(pud_bad(*pud)))
> >                 reutrn NULL;

> Actually, it was Dave who requested them to be taken out of the conditional.

Fair enough. My ack stands, either way!

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-09  8:22 [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings Ard Biesheuvel
  2017-06-09  9:22 ` Mark Rutland
@ 2017-06-09 18:13 ` Laura Abbott
  2017-06-15 21:24 ` Andrew Morton
  2 siblings, 0 replies; 10+ messages in thread
From: Laura Abbott @ 2017-06-09 18:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/09/2017 01:22 AM, Ard Biesheuvel wrote:
> Existing code that uses vmalloc_to_page() may assume that any
> address for which is_vmalloc_addr() returns true may be passed
> into vmalloc_to_page() to retrieve the associated struct page.
> 
> This is not un unreasonable assumption to make, but on architectures
> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
> need to ensure that vmalloc_to_page() does not go off into the weeds
> trying to dereference huge PUDs or PMDs as table entries.
> 
> Given that vmalloc() and vmap() themselves never create huge
> mappings or deal with compound pages at all, there is no correct
> answer in this case, so return NULL instead, and issue a warning.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Laura Abbott <labbott@redhat.com>

> ---
> v5: - fix typo
> 
> v4: - use pud_bad/pmd_bad instead of pud_huge/pmd_huge, which don't require
>       changes to hugetlb.h, and give us what we need on all architectures
>     - move WARN_ON_ONCE() calls out of conditionals
>     - add explanatory comment
> 
>  mm/vmalloc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 34a1c3e46ed7..0fcd371266a4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -287,10 +287,21 @@ struct page *vmalloc_to_page(const void *vmalloc_addr)
>  	if (p4d_none(*p4d))
>  		return NULL;
>  	pud = pud_offset(p4d, addr);
> -	if (pud_none(*pud))
> +
> +	/*
> +	 * Don't dereference bad PUD or PMD (below) entries. This will also
> +	 * identify huge mappings, which we may encounter on architectures
> +	 * that define CONFIG_HAVE_ARCH_HUGE_VMAP=y. Such regions will be
> +	 * identified as vmalloc addresses by is_vmalloc_addr(), but are
> +	 * not [unambiguously] associated with a struct page, so there is
> +	 * no correct value to return for them.
> +	 */
> +	WARN_ON_ONCE(pud_bad(*pud));
> +	if (pud_none(*pud) || pud_bad(*pud))
>  		return NULL;
>  	pmd = pmd_offset(pud, addr);
> -	if (pmd_none(*pmd))
> +	WARN_ON_ONCE(pmd_bad(*pmd));
> +	if (pmd_none(*pmd) || pmd_bad(*pmd))
>  		return NULL;
>  
>  	ptep = pte_offset_map(pmd, addr);
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-09  8:22 [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings Ard Biesheuvel
  2017-06-09  9:22 ` Mark Rutland
  2017-06-09 18:13 ` Laura Abbott
@ 2017-06-15 21:24 ` Andrew Morton
  2017-06-15 22:11   ` Ard Biesheuvel
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2017-06-15 21:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri,  9 Jun 2017 08:22:26 +0000 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> Existing code that uses vmalloc_to_page() may assume that any
> address for which is_vmalloc_addr() returns true may be passed
> into vmalloc_to_page() to retrieve the associated struct page.
> 
> This is not un unreasonable assumption to make, but on architectures
> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
> need to ensure that vmalloc_to_page() does not go off into the weeds
> trying to dereference huge PUDs or PMDs as table entries.
> 
> Given that vmalloc() and vmap() themselves never create huge
> mappings or deal with compound pages at all, there is no correct
> answer in this case, so return NULL instead, and issue a warning.

Is this patch known to fix any current user-visible problem?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-15 21:24 ` Andrew Morton
@ 2017-06-15 22:11   ` Ard Biesheuvel
  2017-06-15 22:16     ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-06-15 22:11 UTC (permalink / raw)
  To: linux-arm-kernel



> On 15 Jun 2017, at 23:24, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> On Fri,  9 Jun 2017 08:22:26 +0000 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> 
>> Existing code that uses vmalloc_to_page() may assume that any
>> address for which is_vmalloc_addr() returns true may be passed
>> into vmalloc_to_page() to retrieve the associated struct page.
>> 
>> This is not un unreasonable assumption to make, but on architectures
>> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
>> need to ensure that vmalloc_to_page() does not go off into the weeds
>> trying to dereference huge PUDs or PMDs as table entries.
>> 
>> Given that vmalloc() and vmap() themselves never create huge
>> mappings or deal with compound pages at all, there is no correct
>> answer in this case, so return NULL instead, and issue a warning.
> 
> Is this patch known to fix any current user-visible problem?

Yes. When reading /proc/kcore on arm64, you will hit an oops as soon as you hit the huge mappings used for the various segments that make up the mapping of vmlinux. With this patch applied, you will no longer hit the oops, but the kcore contents willl be incorrect (these regions will be zeroed out)

We are fixing this for kcore specifically, so it avoids vread() for  those regions. At least one other problematic user exists, i.e., /dev/kmem, but that is currently broken on arm64 for other reasons.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-15 22:11   ` Ard Biesheuvel
@ 2017-06-15 22:16     ` Andrew Morton
  2017-06-15 22:29       ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2017-06-15 22:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 16 Jun 2017 00:11:53 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> 
> 
> > On 15 Jun 2017, at 23:24, Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> >> On Fri,  9 Jun 2017 08:22:26 +0000 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> 
> >> Existing code that uses vmalloc_to_page() may assume that any
> >> address for which is_vmalloc_addr() returns true may be passed
> >> into vmalloc_to_page() to retrieve the associated struct page.
> >> 
> >> This is not un unreasonable assumption to make, but on architectures
> >> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
> >> need to ensure that vmalloc_to_page() does not go off into the weeds
> >> trying to dereference huge PUDs or PMDs as table entries.
> >> 
> >> Given that vmalloc() and vmap() themselves never create huge
> >> mappings or deal with compound pages at all, there is no correct
> >> answer in this case, so return NULL instead, and issue a warning.
> > 
> > Is this patch known to fix any current user-visible problem?
> 
> Yes. When reading /proc/kcore on arm64, you will hit an oops as soon as you hit the huge mappings used for the various segments that make up the mapping of vmlinux. With this patch applied, you will no longer hit the oops, but the kcore contents willl be incorrect (these regions will be zeroed out)
> 
> We are fixing this for kcore specifically, so it avoids vread() for  those regions. At least one other problematic user exists, i.e., /dev/kmem, but that is currently broken on arm64 for other reasons.
> 

Do you have any suggestions regarding which kernel version(s) should
get this patch?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-15 22:16     ` Andrew Morton
@ 2017-06-15 22:29       ` Ard Biesheuvel
  2017-06-16  8:38         ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-06-15 22:29 UTC (permalink / raw)
  To: linux-arm-kernel


> On 16 Jun 2017, at 00:16, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> On Fri, 16 Jun 2017 00:11:53 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> 
>> 
>> 
>>>> On 15 Jun 2017, at 23:24, Andrew Morton <akpm@linux-foundation.org> wrote:
>>>> 
>>>> On Fri,  9 Jun 2017 08:22:26 +0000 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>> 
>>>> Existing code that uses vmalloc_to_page() may assume that any
>>>> address for which is_vmalloc_addr() returns true may be passed
>>>> into vmalloc_to_page() to retrieve the associated struct page.
>>>> 
>>>> This is not un unreasonable assumption to make, but on architectures
>>>> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
>>>> need to ensure that vmalloc_to_page() does not go off into the weeds
>>>> trying to dereference huge PUDs or PMDs as table entries.
>>>> 
>>>> Given that vmalloc() and vmap() themselves never create huge
>>>> mappings or deal with compound pages at all, there is no correct
>>>> answer in this case, so return NULL instead, and issue a warning.
>>> 
>>> Is this patch known to fix any current user-visible problem?
>> 
>> Yes. When reading /proc/kcore on arm64, you will hit an oops as soon as you hit the huge mappings used for the various segments that make up the mapping of vmlinux. With this patch applied, you will no longer hit the oops, but the kcore contents willl be incorrect (these regions will be zeroed out)
>> 
>> We are fixing this for kcore specifically, so it avoids vread() for  those regions. At least one other problematic user exists, i.e., /dev/kmem, but that is currently broken on arm64 for other reasons.
>> 
> 
> Do you have any suggestions regarding which kernel version(s) should
> get this patch?
> 

Good question. v4.6 was the first one to enable the huge vmap feature on arm64 iirc, but that does not necessarily mean it needs to be backported at all imo. What is kcore used for? Production grade stuff?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings
  2017-06-15 22:29       ` Ard Biesheuvel
@ 2017-06-16  8:38         ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2017-06-16  8:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 June 2017 at 00:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 16 Jun 2017, at 00:16, Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>>> On Fri, 16 Jun 2017 00:11:53 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>
>>>
>>>
>>>>> On 15 Jun 2017, at 23:24, Andrew Morton <akpm@linux-foundation.org> wrote:
>>>>>
>>>>> On Fri,  9 Jun 2017 08:22:26 +0000 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>>>
>>>>> Existing code that uses vmalloc_to_page() may assume that any
>>>>> address for which is_vmalloc_addr() returns true may be passed
>>>>> into vmalloc_to_page() to retrieve the associated struct page.
>>>>>
>>>>> This is not un unreasonable assumption to make, but on architectures
>>>>> that have CONFIG_HAVE_ARCH_HUGE_VMAP=y, it no longer holds, and we
>>>>> need to ensure that vmalloc_to_page() does not go off into the weeds
>>>>> trying to dereference huge PUDs or PMDs as table entries.
>>>>>
>>>>> Given that vmalloc() and vmap() themselves never create huge
>>>>> mappings or deal with compound pages at all, there is no correct
>>>>> answer in this case, so return NULL instead, and issue a warning.
>>>>
>>>> Is this patch known to fix any current user-visible problem?
>>>
>>> Yes. When reading /proc/kcore on arm64, you will hit an oops as soon as you hit the huge mappings used for the various segments that make up the mapping of vmlinux. With this patch applied, you will no longer hit the oops, but the kcore contents willl be incorrect (these regions will be zeroed out)
>>>
>>> We are fixing this for kcore specifically, so it avoids vread() for  those regions. At least one other problematic user exists, i.e., /dev/kmem, but that is currently broken on arm64 for other reasons.
>>>
>>
>> Do you have any suggestions regarding which kernel version(s) should
>> get this patch?
>>
>
> Good question. v4.6 was the first one to enable the huge vmap feature on arm64 iirc, but that does not necessarily mean it needs to be backported at all imo. What is kcore used for? Production grade stuff?

In any case, could you perhaps simply queue it for v4.13? If it needs
to go into -stable, we can always do it later.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-06-16  8:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-09  8:22 [PATCH v5] mm: huge-vmap: fail gracefully on unexpected huge vmap mappings Ard Biesheuvel
2017-06-09  9:22 ` Mark Rutland
2017-06-09  9:27   ` Ard Biesheuvel
2017-06-09  9:29     ` Mark Rutland
2017-06-09 18:13 ` Laura Abbott
2017-06-15 21:24 ` Andrew Morton
2017-06-15 22:11   ` Ard Biesheuvel
2017-06-15 22:16     ` Andrew Morton
2017-06-15 22:29       ` Ard Biesheuvel
2017-06-16  8:38         ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).