diff mbox series

mm: Fix modifying of page protection by insert_pfn()

Message ID 20190311084537.16029-1-jack@suse.cz (mailing list archive)
State New, archived
Headers show
Series mm: Fix modifying of page protection by insert_pfn() | expand

Commit Message

Jan Kara March 11, 2019, 8:45 a.m. UTC
Aneesh has reported that PPC triggers the following warning when
excercising DAX code:

[c00000000007610c] set_pte_at+0x3c/0x190
LR [c000000000378628] insert_pfn+0x208/0x280
Call Trace:
[c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
[c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
[c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
[c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
[c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
[c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
[c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
[c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
[c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18

Now that is WARN_ON in set_pte_at which is

        VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));

The problem is that on some architectures set_pte_at() cannot cope with
a situation where there is already some (different) valid entry present.

Use ptep_set_access_flags() instead to modify the pfn which is built to
deal with modifying existing PTE.

CC: stable@vger.kernel.org
Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 mm/memory.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Aneesh Kumar K.V March 11, 2019, 11:59 a.m. UTC | #1
Jan Kara <jack@suse.cz> writes:

> Aneesh has reported that PPC triggers the following warning when
> excercising DAX code:
>
> [c00000000007610c] set_pte_at+0x3c/0x190
> LR [c000000000378628] insert_pfn+0x208/0x280
> Call Trace:
> [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
>
> Now that is WARN_ON in set_pte_at which is
>
>         VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
>
> The problem is that on some architectures set_pte_at() cannot cope with
> a situation where there is already some (different) valid entry present.
>
> Use ptep_set_access_flags() instead to modify the pfn which is built to
> deal with modifying existing PTE.
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> CC: stable@vger.kernel.org
> Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  mm/memory.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 47fe250307c7..ab650c21bccd 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1549,10 +1549,12 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>  				WARN_ON_ONCE(!is_zero_pfn(pte_pfn(*pte)));
>  				goto out_unlock;
>  			}
> -			entry = *pte;
> -			goto out_mkwrite;
> -		} else
> -			goto out_unlock;
> +			entry = pte_mkyoung(*pte);
> +			entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> +			if (ptep_set_access_flags(vma, addr, pte, entry, 1))
> +				update_mmu_cache(vma, addr, pte);
> +		}
> +		goto out_unlock;
>  	}
>  
>  	/* Ok, finally just insert the thing.. */
> @@ -1561,7 +1563,6 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>  	else
>  		entry = pte_mkspecial(pfn_t_pte(pfn, prot));
>  
> -out_mkwrite:
>  	if (mkwrite) {
>  		entry = pte_mkyoung(entry);
>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> -- 
> 2.16.4
Dan Williams March 11, 2019, 5:22 p.m. UTC | #2
On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
>
> Aneesh has reported that PPC triggers the following warning when
> excercising DAX code:
>
> [c00000000007610c] set_pte_at+0x3c/0x190
> LR [c000000000378628] insert_pfn+0x208/0x280
> Call Trace:
> [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
>
> Now that is WARN_ON in set_pte_at which is
>
>         VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
>
> The problem is that on some architectures set_pte_at() cannot cope with
> a situation where there is already some (different) valid entry present.
>
> Use ptep_set_access_flags() instead to modify the pfn which is built to
> deal with modifying existing PTE.
>
> CC: stable@vger.kernel.org
> Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Dan Williams <dan.j.williams@intel.com>

Andrew, can you pick this up?
Sasha Levin March 25, 2019, 12:38 a.m. UTC | #3
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: b2770da64254 mm: add vm_insert_mixed_mkwrite().

The bot has tested the following trees: v5.0.3, v4.19.30, v4.14.107.

v5.0.3: Build OK!
v4.19.30: Failed to apply! Possible dependencies:
    f2c57d91b0d9 ("mm: Fix warning in insert_pfn()")

v4.14.107: Failed to apply! Possible dependencies:
    f2c57d91b0d9 ("mm: Fix warning in insert_pfn()")


How should we proceed with this patch?

--
Thanks,
Sasha
Jan Kara March 25, 2019, 5:02 p.m. UTC | #4
On Mon 25-03-19 00:38:20, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: b2770da64254 mm: add vm_insert_mixed_mkwrite().
> 
> The bot has tested the following trees: v5.0.3, v4.19.30, v4.14.107.
> 
> v5.0.3: Build OK!
> v4.19.30: Failed to apply! Possible dependencies:
>     f2c57d91b0d9 ("mm: Fix warning in insert_pfn()")
> 
> v4.14.107: Failed to apply! Possible dependencies:
>     f2c57d91b0d9 ("mm: Fix warning in insert_pfn()")
> 
> 
> How should we proceed with this patch?

I'd say apply also f2c57d91b0d9 to both trees. Nice automation BTW :).

								Honza
Jan Kara March 27, 2019, 5:33 p.m. UTC | #5
On Mon 11-03-19 10:22:44, Dan Williams wrote:
> On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
> >
> > Aneesh has reported that PPC triggers the following warning when
> > excercising DAX code:
> >
> > [c00000000007610c] set_pte_at+0x3c/0x190
> > LR [c000000000378628] insert_pfn+0x208/0x280
> > Call Trace:
> > [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> > [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> > [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> > [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> > [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> > [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> > [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> > [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> > [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
> >
> > Now that is WARN_ON in set_pte_at which is
> >
> >         VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
> >
> > The problem is that on some architectures set_pte_at() cannot cope with
> > a situation where there is already some (different) valid entry present.
> >
> > Use ptep_set_access_flags() instead to modify the pfn which is built to
> > deal with modifying existing PTE.
> >
> > CC: stable@vger.kernel.org
> > Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> > Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> 
> Andrew, can you pick this up?

Andrew, ping?

								Honza
Andrew Morton March 27, 2019, 9:14 p.m. UTC | #6
On Wed, 27 Mar 2019 18:33:32 +0100 Jan Kara <jack@suse.cz> wrote:

> On Mon 11-03-19 10:22:44, Dan Williams wrote:
> > On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
> > >
> > > Aneesh has reported that PPC triggers the following warning when
> > > excercising DAX code:
> > >
> > > [c00000000007610c] set_pte_at+0x3c/0x190
> > > LR [c000000000378628] insert_pfn+0x208/0x280
> > > Call Trace:
> > > [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> > > [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> > > [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> > > [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> > > [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> > > [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> > > [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> > > [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> > > [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
> > >
> > > Now that is WARN_ON in set_pte_at which is
> > >
> > >         VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
> > >
> > > The problem is that on some architectures set_pte_at() cannot cope with
> > > a situation where there is already some (different) valid entry present.
> > >
> > > Use ptep_set_access_flags() instead to modify the pfn which is built to
> > > deal with modifying existing PTE.
> > >
> > > CC: stable@vger.kernel.org
> > > Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> > > Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > 
> > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > 
> > Andrew, can you pick this up?
> 
> Andrew, ping?

I merged this a couple of weeks ago and it's in the queue for 5.1.
Aneesh Kumar K.V March 28, 2019, 3:18 a.m. UTC | #7
On 3/28/19 2:44 AM, Andrew Morton wrote:
> On Wed, 27 Mar 2019 18:33:32 +0100 Jan Kara <jack@suse.cz> wrote:
> 
>> On Mon 11-03-19 10:22:44, Dan Williams wrote:
>>> On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
>>>>
>>>> Aneesh has reported that PPC triggers the following warning when
>>>> excercising DAX code:
>>>>
>>>> [c00000000007610c] set_pte_at+0x3c/0x190
>>>> LR [c000000000378628] insert_pfn+0x208/0x280
>>>> Call Trace:
>>>> [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
>>>> [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
>>>> [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
>>>> [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
>>>> [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
>>>> [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
>>>> [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
>>>> [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
>>>> [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
>>>>
>>>> Now that is WARN_ON in set_pte_at which is
>>>>
>>>>          VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
>>>>
>>>> The problem is that on some architectures set_pte_at() cannot cope with
>>>> a situation where there is already some (different) valid entry present.
>>>>
>>>> Use ptep_set_access_flags() instead to modify the pfn which is built to
>>>> deal with modifying existing PTE.
>>>>
>>>> CC: stable@vger.kernel.org
>>>> Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
>>>> Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
>>>> Signed-off-by: Jan Kara <jack@suse.cz>
>>>
>>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>>>
>>> Andrew, can you pick this up?
>>
>> Andrew, ping?
> 
> I merged this a couple of weeks ago and it's in the queue for 5.1.
> 

I noticed that we need similar change for pmd and pud updates. I will 
send a patch for that.

-aneesh
Jan Kara March 28, 2019, 9:02 a.m. UTC | #8
On Wed 27-03-19 14:14:14, Andrew Morton wrote:
> On Wed, 27 Mar 2019 18:33:32 +0100 Jan Kara <jack@suse.cz> wrote:
> 
> > On Mon 11-03-19 10:22:44, Dan Williams wrote:
> > > On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
> > > >
> > > > Aneesh has reported that PPC triggers the following warning when
> > > > excercising DAX code:
> > > >
> > > > [c00000000007610c] set_pte_at+0x3c/0x190
> > > > LR [c000000000378628] insert_pfn+0x208/0x280
> > > > Call Trace:
> > > > [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> > > > [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> > > > [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> > > > [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> > > > [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> > > > [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> > > > [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> > > > [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> > > > [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
> > > >
> > > > Now that is WARN_ON in set_pte_at which is
> > > >
> > > >         VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
> > > >
> > > > The problem is that on some architectures set_pte_at() cannot cope with
> > > > a situation where there is already some (different) valid entry present.
> > > >
> > > > Use ptep_set_access_flags() instead to modify the pfn which is built to
> > > > deal with modifying existing PTE.
> > > >
> > > > CC: stable@vger.kernel.org
> > > > Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> > > > Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > 
> > > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > > 
> > > Andrew, can you pick this up?
> > 
> > Andrew, ping?
> 
> I merged this a couple of weeks ago and it's in the queue for 5.1.

Ah, sorry. I didn't find any email about this in my archives. Not sure what
happened. Thanks for merging the patch!

								Honza
Jan Kara March 28, 2019, 9:02 a.m. UTC | #9
On Thu 28-03-19 08:48:19, Aneesh Kumar K.V wrote:
> On 3/28/19 2:44 AM, Andrew Morton wrote:
> > On Wed, 27 Mar 2019 18:33:32 +0100 Jan Kara <jack@suse.cz> wrote:
> > 
> > > On Mon 11-03-19 10:22:44, Dan Williams wrote:
> > > > On Mon, Mar 11, 2019 at 1:45 AM Jan Kara <jack@suse.cz> wrote:
> > > > > 
> > > > > Aneesh has reported that PPC triggers the following warning when
> > > > > excercising DAX code:
> > > > > 
> > > > > [c00000000007610c] set_pte_at+0x3c/0x190
> > > > > LR [c000000000378628] insert_pfn+0x208/0x280
> > > > > Call Trace:
> > > > > [c0000002125df980] [8000000000000104] 0x8000000000000104 (unreliable)
> > > > > [c0000002125df9c0] [c000000000378488] insert_pfn+0x68/0x280
> > > > > [c0000002125dfa30] [c0000000004a5494] dax_iomap_pte_fault.isra.7+0x734/0xa40
> > > > > [c0000002125dfb50] [c000000000627250] __xfs_filemap_fault+0x280/0x2d0
> > > > > [c0000002125dfbb0] [c000000000373abc] do_wp_page+0x48c/0xa40
> > > > > [c0000002125dfc00] [c000000000379170] __handle_mm_fault+0x8d0/0x1fd0
> > > > > [c0000002125dfd00] [c00000000037a9b0] handle_mm_fault+0x140/0x250
> > > > > [c0000002125dfd40] [c000000000074bb0] __do_page_fault+0x300/0xd60
> > > > > [c0000002125dfe20] [c00000000000acf4] handle_page_fault+0x18
> > > > > 
> > > > > Now that is WARN_ON in set_pte_at which is
> > > > > 
> > > > >          VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
> > > > > 
> > > > > The problem is that on some architectures set_pte_at() cannot cope with
> > > > > a situation where there is already some (different) valid entry present.
> > > > > 
> > > > > Use ptep_set_access_flags() instead to modify the pfn which is built to
> > > > > deal with modifying existing PTE.
> > > > > 
> > > > > CC: stable@vger.kernel.org
> > > > > Fixes: b2770da64254 "mm: add vm_insert_mixed_mkwrite()"
> > > > > Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > > > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > > 
> > > > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > > > 
> > > > Andrew, can you pick this up?
> > > 
> > > Andrew, ping?
> > 
> > I merged this a couple of weeks ago and it's in the queue for 5.1.
> > 
> 
> I noticed that we need similar change for pmd and pud updates. I will send a
> patch for that.

Yes, it is needed there as well. Thanks for fixing this.

								Honza
diff mbox series

Patch

diff --git a/mm/memory.c b/mm/memory.c
index 47fe250307c7..ab650c21bccd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1549,10 +1549,12 @@  static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 				WARN_ON_ONCE(!is_zero_pfn(pte_pfn(*pte)));
 				goto out_unlock;
 			}
-			entry = *pte;
-			goto out_mkwrite;
-		} else
-			goto out_unlock;
+			entry = pte_mkyoung(*pte);
+			entry = maybe_mkwrite(pte_mkdirty(entry), vma);
+			if (ptep_set_access_flags(vma, addr, pte, entry, 1))
+				update_mmu_cache(vma, addr, pte);
+		}
+		goto out_unlock;
 	}
 
 	/* Ok, finally just insert the thing.. */
@@ -1561,7 +1563,6 @@  static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 	else
 		entry = pte_mkspecial(pfn_t_pte(pfn, prot));
 
-out_mkwrite:
 	if (mkwrite) {
 		entry = pte_mkyoung(entry);
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);