[PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements

David Hildenbrand posted 1 patch 10 months, 1 week ago
arch/x86/mm/pat/memtype.c | 4 +++-
include/linux/pgtable.h   | 5 ++++-
mm/memory.c               | 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)
[PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by David Hildenbrand 10 months, 1 week ago
We got a late smatch warning and some additional review feedback.

	smatch warnings:
	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.

We actually use the pfn only when it is properly initialized; however,
we may pass an uninitialized value to a function -- although it will not
use it that likely still is UB in C.

Fix it by always initializing pfn when track_pfn_copy() returns 0 --
just as we document ("On success, stores the pfn to be passed to
untrack_pfn_copy()"). In addition, to avoid further wrong smatch
warnings, just initialize pfn = 0 in the caller as well.

While at it, clarify the doc of untrack_pfn_copy(), that internal checks
make sure if we actually have to untrack anything.

Fixes: dc84bc2aba85 ("x86/mm/pat: Fix VM_PAT handling when fork() fails in copy_page_range()")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202503270941.IFILyNCX-lkp@intel.com/
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Rik van Riel <riel@surriel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/mm/pat/memtype.c | 4 +++-
 include/linux/pgtable.h   | 5 ++++-
 mm/memory.c               | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
index 72d8cbc611583..9ad3e5b055d8a 100644
--- a/arch/x86/mm/pat/memtype.c
+++ b/arch/x86/mm/pat/memtype.c
@@ -992,8 +992,10 @@ int track_pfn_copy(struct vm_area_struct *dst_vma,
 	pgprot_t pgprot;
 	int rc;
 
-	if (!(src_vma->vm_flags & VM_PAT))
+	if (!(src_vma->vm_flags & VM_PAT)) {
+		*pfn = 0;
 		return 0;
+	}
 
 	/*
 	 * Duplicate the PAT information for the dst VMA based on the src
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index e2b705c149454..9457064292141 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1517,12 +1517,15 @@ static inline void track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot,
 static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
 		struct vm_area_struct *src_vma, unsigned long *pfn)
 {
+	*pfn = 0;
 	return 0;
 }
 
 /*
  * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
- * copy_page_range(), but after track_pfn_copy() was already called.
+ * copy_page_range(), but after track_pfn_copy() was already called. Can
+ * be called even if track_pfn_copy() did not actually track anything:
+ * handled internally.
  */
 static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
 		unsigned long pfn)
diff --git a/mm/memory.c b/mm/memory.c
index 2d8c265fc7d60..1a35165622e1c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
 	struct mm_struct *dst_mm = dst_vma->vm_mm;
 	struct mm_struct *src_mm = src_vma->vm_mm;
 	struct mmu_notifier_range range;
-	unsigned long next, pfn;
+	unsigned long next, pfn = 0;
 	bool is_cow;
 	int ret;
 
-- 
2.48.1
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Ingo Molnar 10 months ago
* David Hildenbrand <david@redhat.com> wrote:

> We got a late smatch warning and some additional review feedback.
> 
> 	smatch warnings:
> 	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.

> -	if (!(src_vma->vm_flags & VM_PAT))
> +	if (!(src_vma->vm_flags & VM_PAT)) {
> +		*pfn = 0;
>  		return 0;
> +	}

>  static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>  		struct vm_area_struct *src_vma, unsigned long *pfn)
>  {
> +	*pfn = 0;
>  	return 0;
>  }

That's way too ugly. There's nothing wrong with not touching 'pfn' in 
the error path: in fact it's pretty standard API where output pointers 
may not get set on errors.

If Smatch has a problem with it, Smatch should be fixed, or the false 
positive warning should be worked around by initializing 'pfn' in the 
callers.

Thanks,

	Ingo
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by David Hildenbrand 10 months ago
On 06.04.25 19:28, Ingo Molnar wrote:
> 
> * David Hildenbrand <david@redhat.com> wrote:
> 
>> We got a late smatch warning and some additional review feedback.
>>
>> 	smatch warnings:
>> 	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.
> 
>> -	if (!(src_vma->vm_flags & VM_PAT))
>> +	if (!(src_vma->vm_flags & VM_PAT)) {
>> +		*pfn = 0;
>>   		return 0;
>> +	}
> 
>>   static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>>   		struct vm_area_struct *src_vma, unsigned long *pfn)
>>   {
>> +	*pfn = 0;
>>   	return 0;
>>   }
> 
> That's way too ugly. There's nothing wrong with not touching 'pfn' in
> the error path: in fact it's pretty standard API where output pointers
> may not get set on errors.

We're not concerned about the error path, though.

> 
> If Smatch has a problem with it, Smatch should be fixed, or the false
> positive warning should be worked around by initializing 'pfn' in the
> callers.

We could adjust the documentation of track_pfn_copy, to end up with the
following:

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index e2b705c149454..b50447ef1c921 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1511,8 +1511,9 @@ static inline void track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot,
  
  /*
   * track_pfn_copy is called when a VM_PFNMAP VMA is about to get the page
- * tables copied during copy_page_range(). On success, stores the pfn to be
- * passed to untrack_pfn_copy().
+ * tables copied during copy_page_range(). Will store the pfn to be
+ * passed to untrack_pfn_copy() only if there is something to be untracked.
+ * Callers should initialize the pfn to 0.
   */
  static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
                 struct vm_area_struct *src_vma, unsigned long *pfn)
@@ -1522,7 +1523,9 @@ static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
  
  /*
   * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
- * copy_page_range(), but after track_pfn_copy() was already called.
+ * copy_page_range(), but after track_pfn_copy() was already called. Can
+ * be called even if track_pfn_copy() did not actually track anything:
+ * handled internally.
   */
  static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
                 unsigned long pfn)
diff --git a/mm/memory.c b/mm/memory.c
index 2d8c265fc7d60..1a35165622e1c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
         struct mm_struct *dst_mm = dst_vma->vm_mm;
         struct mm_struct *src_mm = src_vma->vm_mm;
         struct mmu_notifier_range range;
-       unsigned long next, pfn;
+       unsigned long next, pfn = 0;
         bool is_cow;
         int ret;


-- 
Cheers,

David / dhildenb
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Ingo Molnar 10 months ago
* David Hildenbrand <david@redhat.com> wrote:

> On 06.04.25 19:28, Ingo Molnar wrote:
> > 
> > * David Hildenbrand <david@redhat.com> wrote:
> > 
> > > We got a late smatch warning and some additional review feedback.
> > > 
> > > 	smatch warnings:
> > > 	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.
> > 
> > > -	if (!(src_vma->vm_flags & VM_PAT))
> > > +	if (!(src_vma->vm_flags & VM_PAT)) {
> > > +		*pfn = 0;
> > >   		return 0;
> > > +	}
> > 
> > >   static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
> > >   		struct vm_area_struct *src_vma, unsigned long *pfn)
> > >   {
> > > +	*pfn = 0;
> > >   	return 0;
> > >   }
> > 
> > That's way too ugly. There's nothing wrong with not touching 'pfn' 
> > in the error path: in fact it's pretty standard API where output 
> > pointers may not get set on errors.
> 
> We're not concerned about the error path, though.

Sorry, indeed, not an error path, but the !VM_PAT path above - but 
still a similar argument applies IMHO.

> > If Smatch has a problem with it, Smatch should be fixed, or the false
> > positive warning should be worked around by initializing 'pfn' in the
> > callers.
> 
> We could adjust the documentation of track_pfn_copy, to end up with the
> following:
> 
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index e2b705c149454..b50447ef1c921 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1511,8 +1511,9 @@ static inline void track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot,
>  /*
>   * track_pfn_copy is called when a VM_PFNMAP VMA is about to get the page
> - * tables copied during copy_page_range(). On success, stores the pfn to be
> - * passed to untrack_pfn_copy().
> + * tables copied during copy_page_range(). Will store the pfn to be
> + * passed to untrack_pfn_copy() only if there is something to be untracked.
> + * Callers should initialize the pfn to 0.
>   */
>  static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>                 struct vm_area_struct *src_vma, unsigned long *pfn)
> @@ -1522,7 +1523,9 @@ static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>  /*
>   * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
> - * copy_page_range(), but after track_pfn_copy() was already called.
> + * copy_page_range(), but after track_pfn_copy() was already called. Can
> + * be called even if track_pfn_copy() did not actually track anything:
> + * handled internally.
>   */
>  static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
>                 unsigned long pfn)
> diff --git a/mm/memory.c b/mm/memory.c
> index 2d8c265fc7d60..1a35165622e1c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>         struct mm_struct *dst_mm = dst_vma->vm_mm;
>         struct mm_struct *src_mm = src_vma->vm_mm;
>         struct mmu_notifier_range range;
> -       unsigned long next, pfn;
> +       unsigned long next, pfn = 0;

Ack.

I hate it how uninitialized variables are even a thing in C, and why 
there's no compiler switch to turn it off for the kernel. (At least for 
non-struct variables. Even for structs I would zero-initialize and 
*maybe* allow a non-initialized opt-in for cases where it matters. It 
matters in very few cases in praxis. And don't get me started about the 
stupidity that is to not initialize holes in struct members ...)

Over the decades we've lived through numerous nasty bugs for very 
little tangible code generation benefits.

Thanks,

	Ingo
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Dan Carpenter 10 months ago
On Mon, Apr 07, 2025 at 06:50:43PM +0200, Ingo Molnar wrote:
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 2d8c265fc7d60..1a35165622e1c 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
> >         struct mm_struct *dst_mm = dst_vma->vm_mm;
> >         struct mm_struct *src_mm = src_vma->vm_mm;
> >         struct mmu_notifier_range range;
> > -       unsigned long next, pfn;
> > +       unsigned long next, pfn = 0;
> 
> Ack.
> 
> I hate it how uninitialized variables are even a thing in C, and why
> there's no compiler switch to turn it off for the kernel. (At least for
> non-struct variables. Even for structs I would zero-initialize and
> *maybe* allow a non-initialized opt-in for cases where it matters. It
> matters in very few cases in praxis. And don't get me started about the
> stupidity that is to not initialize holes in struct members ...)

Everyone sane uses CONFIG_INIT_STACK_ALL_ZERO these days.

regards,
dan carpenter
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Ingo Molnar 10 months ago
* Dan Carpenter <dan.carpenter@linaro.org> wrote:

> > least for non-struct variables. Even for structs I would 
> > zero-initialize and *maybe* allow a non-initialized opt-in for 
> > cases where it matters. It matters in very few cases in praxis. And 
> > don't get me started about the stupidity that is to not initialize 
> > holes in struct members ...)
> 
> Everyone sane uses CONFIG_INIT_STACK_ALL_ZERO these days.

Good, although why is this compiler option named so weirdly in Clang:

  CC_AUTO_VAR_INIT_ZERO_ENABLER := -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

Hopefully it is named thusly because Clang has adopted GCC's 
-ftrivial-auto-var-init=zero?

Thanks,

	Ingo
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Nathan Chancellor 10 months ago
On Mon, Apr 07, 2025 at 08:59:44PM +0200, Ingo Molnar wrote:
> Good, although why is this compiler option named so weirdly in Clang:
> 
>   CC_AUTO_VAR_INIT_ZERO_ENABLER := -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> 
> Hopefully it is named thusly because Clang has adopted GCC's 
> -ftrivial-auto-var-init=zero?

Clang did -ftrivial-auto-var-init first, where the original author added
both pattern and zero but intended to remove zero once pattern has been
optimized enough compared to zero (if I remember and understand
correctly), so the "enabler" flag was added to try and make that clear.
Eventually, Kees leveraged both Linus's stated desire for initializing
stack variables [1] and GCC 12 landing -ftrivial-auto-var-init=zero
without a separate enable option to deprecate the "enabler" flag in
clang 16 [2] and remove it altogether in clang 18 [3].

[1]: https://lore.kernel.org/CAHk-=wgTM+cN7zyUZacGQDv3DuuoA4LORNPWgb1Y_Z1p4iedNQ@mail.gmail.com/
[2]: https://github.com/llvm/llvm-project/commit/aef03c9b3bed5cef5a1940774b80128aefcb4095
[3]: https://github.com/llvm/llvm-project/commit/00e54d04ae2802d498741097d4b83e898bc99c5b

Cheers,
Nathan
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Ingo Molnar 10 months ago
* Nathan Chancellor <nathan@kernel.org> wrote:

> On Mon, Apr 07, 2025 at 08:59:44PM +0200, Ingo Molnar wrote:
> > Good, although why is this compiler option named so weirdly in Clang:
> > 
> >   CC_AUTO_VAR_INIT_ZERO_ENABLER := -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> > 
> > Hopefully it is named thusly because Clang has adopted GCC's 
> > -ftrivial-auto-var-init=zero?
> 
> Clang did -ftrivial-auto-var-init first, where the original author added
> both pattern and zero but intended to remove zero once pattern has been
> optimized enough compared to zero (if I remember and understand
> correctly), so the "enabler" flag was added to try and make that clear.
> Eventually, Kees leveraged both Linus's stated desire for initializing
> stack variables [1] and GCC 12 landing -ftrivial-auto-var-init=zero
> without a separate enable option to deprecate the "enabler" flag in
> clang 16 [2] and remove it altogether in clang 18 [3].
> 
> [1]: https://lore.kernel.org/CAHk-=wgTM+cN7zyUZacGQDv3DuuoA4LORNPWgb1Y_Z1p4iedNQ@mail.gmail.com/
> [2]: https://github.com/llvm/llvm-project/commit/aef03c9b3bed5cef5a1940774b80128aefcb4095
> [3]: https://github.com/llvm/llvm-project/commit/00e54d04ae2802d498741097d4b83e898bc99c5b

Cool, thanks for the explanation!

Thanks,

	Ingo
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by David Hildenbrand 10 months ago
On 07.04.25 18:50, Ingo Molnar wrote:
> 
> * David Hildenbrand <david@redhat.com> wrote:
> 
>> On 06.04.25 19:28, Ingo Molnar wrote:
>>>
>>> * David Hildenbrand <david@redhat.com> wrote:
>>>
>>>> We got a late smatch warning and some additional review feedback.
>>>>
>>>> 	smatch warnings:
>>>> 	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.
>>>
>>>> -	if (!(src_vma->vm_flags & VM_PAT))
>>>> +	if (!(src_vma->vm_flags & VM_PAT)) {
>>>> +		*pfn = 0;
>>>>    		return 0;
>>>> +	}
>>>
>>>>    static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>>>>    		struct vm_area_struct *src_vma, unsigned long *pfn)
>>>>    {
>>>> +	*pfn = 0;
>>>>    	return 0;
>>>>    }
>>>
>>> That's way too ugly. There's nothing wrong with not touching 'pfn'
>>> in the error path: in fact it's pretty standard API where output
>>> pointers may not get set on errors.
>>
>> We're not concerned about the error path, though.
> 
> Sorry, indeed, not an error path, but the !VM_PAT path above - but
> still a similar argument applies IMHO.
> 
>>> If Smatch has a problem with it, Smatch should be fixed, or the false
>>> positive warning should be worked around by initializing 'pfn' in the
>>> callers.
>>
>> We could adjust the documentation of track_pfn_copy, to end up with the
>> following:
>>
>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>> index e2b705c149454..b50447ef1c921 100644
>> --- a/include/linux/pgtable.h
>> +++ b/include/linux/pgtable.h
>> @@ -1511,8 +1511,9 @@ static inline void track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot,
>>   /*
>>    * track_pfn_copy is called when a VM_PFNMAP VMA is about to get the page
>> - * tables copied during copy_page_range(). On success, stores the pfn to be
>> - * passed to untrack_pfn_copy().
>> + * tables copied during copy_page_range(). Will store the pfn to be
>> + * passed to untrack_pfn_copy() only if there is something to be untracked.
>> + * Callers should initialize the pfn to 0.
>>    */
>>   static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>>                  struct vm_area_struct *src_vma, unsigned long *pfn)
>> @@ -1522,7 +1523,9 @@ static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>>   /*
>>    * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
>> - * copy_page_range(), but after track_pfn_copy() was already called.
>> + * copy_page_range(), but after track_pfn_copy() was already called. Can
>> + * be called even if track_pfn_copy() did not actually track anything:
>> + * handled internally.
>>    */
>>   static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
>>                  unsigned long pfn)
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 2d8c265fc7d60..1a35165622e1c 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>>          struct mm_struct *dst_mm = dst_vma->vm_mm;
>>          struct mm_struct *src_mm = src_vma->vm_mm;
>>          struct mmu_notifier_range range;
>> -       unsigned long next, pfn;
>> +       unsigned long next, pfn = 0;
> 
> Ack.
> 
> I hate it how uninitialized variables are even a thing in C, and why
> there's no compiler switch to turn it off for the kernel. (At least for
> non-struct variables. Even for structs I would zero-initialize and
> *maybe* allow a non-initialized opt-in for cases where it matters. It
> matters in very few cases in praxis. And don't get me started about the
> stupidity that is to not initialize holes in struct members ...)
> 
> Over the decades we've lived through numerous nasty bugs for very
> little tangible code generation benefits.

Ok, let me resend with that. (I'll still tag it as a fix due do the 
weird UB scenario when passing uninitialized values to a non-inline 
function ...)

-- 
Cheers,

David / dhildenb
Re: [PATCH v1] x86/mm/pat: (un)track_pfn_copy() fix + improvements
Posted by Lorenzo Stoakes 10 months, 1 week ago
On Fri, Apr 04, 2025 at 02:49:31PM +0200, David Hildenbrand wrote:
> We got a late smatch warning and some additional review feedback.
>
> 	smatch warnings:
> 	mm/memory.c:1428 copy_page_range() error: uninitialized symbol 'pfn'.
>
> We actually use the pfn only when it is properly initialized; however,
> we may pass an uninitialized value to a function -- although it will not
> use it that likely still is UB in C.
>
> Fix it by always initializing pfn when track_pfn_copy() returns 0 --
> just as we document ("On success, stores the pfn to be passed to
> untrack_pfn_copy()"). In addition, to avoid further wrong smatch
> warnings, just initialize pfn = 0 in the caller as well.
>
> While at it, clarify the doc of untrack_pfn_copy(), that internal checks
> make sure if we actually have to untrack anything.
>
> Fixes: dc84bc2aba85 ("x86/mm/pat: Fix VM_PAT handling when fork() fails in copy_page_range()")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/r/202503270941.IFILyNCX-lkp@intel.com/
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Rik van Riel <riel@surriel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>

LGTM,

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  arch/x86/mm/pat/memtype.c | 4 +++-
>  include/linux/pgtable.h   | 5 ++++-
>  mm/memory.c               | 2 +-
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
> index 72d8cbc611583..9ad3e5b055d8a 100644
> --- a/arch/x86/mm/pat/memtype.c
> +++ b/arch/x86/mm/pat/memtype.c
> @@ -992,8 +992,10 @@ int track_pfn_copy(struct vm_area_struct *dst_vma,
>  	pgprot_t pgprot;
>  	int rc;
>
> -	if (!(src_vma->vm_flags & VM_PAT))
> +	if (!(src_vma->vm_flags & VM_PAT)) {
> +		*pfn = 0;
>  		return 0;
> +	}
>
>  	/*
>  	 * Duplicate the PAT information for the dst VMA based on the src
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index e2b705c149454..9457064292141 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1517,12 +1517,15 @@ static inline void track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot,
>  static inline int track_pfn_copy(struct vm_area_struct *dst_vma,
>  		struct vm_area_struct *src_vma, unsigned long *pfn)
>  {
> +	*pfn = 0;
>  	return 0;
>  }
>
>  /*
>   * untrack_pfn_copy is called when a VM_PFNMAP VMA failed to copy during
> - * copy_page_range(), but after track_pfn_copy() was already called.
> + * copy_page_range(), but after track_pfn_copy() was already called. Can
> + * be called even if track_pfn_copy() did not actually track anything:
> + * handled internally.
>   */
>  static inline void untrack_pfn_copy(struct vm_area_struct *dst_vma,
>  		unsigned long pfn)
> diff --git a/mm/memory.c b/mm/memory.c
> index 2d8c265fc7d60..1a35165622e1c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1361,7 +1361,7 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)
>  	struct mm_struct *dst_mm = dst_vma->vm_mm;
>  	struct mm_struct *src_mm = src_vma->vm_mm;
>  	struct mmu_notifier_range range;
> -	unsigned long next, pfn;
> +	unsigned long next, pfn = 0;
>  	bool is_cow;
>  	int ret;
>
> --
> 2.48.1
>