* [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
@ 2022-08-03 22:03 Peter Xu
2022-08-04 9:02 ` Vlastimil Babka (SUSE)
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Peter Xu @ 2022-08-03 22:03 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: peterx, Yang Shi, Andrew Morton, Nadav Amit, David Hildenbrand,
Konstantin Khlebnikov
These bits should only be valid when the ptes are present. Introducing two
booleans for it and set it to false when !pte_present().
Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
Fixes: b1d4d9e0cbd0 ("proc/smaps: carefully handle migration entries", 2012-05-31)
Signed-off-by: Peter Xu <peterx@redhat.com>
---
fs/proc/task_mmu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 9913f3be9fd2..482f91577f8c 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
struct vm_area_struct *vma = walk->vma;
bool locked = !!(vma->vm_flags & VM_LOCKED);
struct page *page = NULL;
- bool migration = false;
+ bool migration = false, young = false, dirty = false;
if (pte_present(*pte)) {
page = vm_normal_page(vma, addr, *pte);
+ young = pte_young(*pte);
+ dirty = pte_dirty(*pte);
} else if (is_swap_pte(*pte)) {
swp_entry_t swpent = pte_to_swp_entry(*pte);
@@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
if (!page)
return;
- smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
- locked, migration);
+ smaps_account(mss, page, false, young, dirty, locked, migration);
}
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
2022-08-03 22:03 [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent Peter Xu
@ 2022-08-04 9:02 ` Vlastimil Babka (SUSE)
2022-08-04 11:01 ` David Hildenbrand
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vlastimil Babka (SUSE) @ 2022-08-04 9:02 UTC (permalink / raw)
To: Peter Xu, linux-kernel, linux-mm
Cc: Yang Shi, Andrew Morton, Nadav Amit, David Hildenbrand,
Konstantin Khlebnikov
On 8/4/22 00:03, Peter Xu wrote:
> These bits should only be valid when the ptes are present. Introducing two
> booleans for it and set it to false when !pte_present().
>
> Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Fixes: b1d4d9e0cbd0 ("proc/smaps: carefully handle migration entries", 2012-05-31)
> Signed-off-by: Peter Xu <peterx@redhat.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> fs/proc/task_mmu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 9913f3be9fd2..482f91577f8c 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> struct vm_area_struct *vma = walk->vma;
> bool locked = !!(vma->vm_flags & VM_LOCKED);
> struct page *page = NULL;
> - bool migration = false;
> + bool migration = false, young = false, dirty = false;
>
> if (pte_present(*pte)) {
> page = vm_normal_page(vma, addr, *pte);
> + young = pte_young(*pte);
> + dirty = pte_dirty(*pte);
> } else if (is_swap_pte(*pte)) {
> swp_entry_t swpent = pte_to_swp_entry(*pte);
>
> @@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> if (!page)
> return;
>
> - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
> - locked, migration);
> + smaps_account(mss, page, false, young, dirty, locked, migration);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
2022-08-03 22:03 [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent Peter Xu
2022-08-04 9:02 ` Vlastimil Babka (SUSE)
@ 2022-08-04 11:01 ` David Hildenbrand
2022-08-04 17:43 ` Yang Shi
2022-08-04 22:40 ` Andrew Morton
3 siblings, 0 replies; 6+ messages in thread
From: David Hildenbrand @ 2022-08-04 11:01 UTC (permalink / raw)
To: Peter Xu, linux-kernel, linux-mm
Cc: Yang Shi, Andrew Morton, Nadav Amit, Konstantin Khlebnikov
On 04.08.22 00:03, Peter Xu wrote:
> These bits should only be valid when the ptes are present. Introducing two
> booleans for it and set it to false when !pte_present().
>
> Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Fixes: b1d4d9e0cbd0 ("proc/smaps: carefully handle migration entries", 2012-05-31)
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> fs/proc/task_mmu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 9913f3be9fd2..482f91577f8c 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> struct vm_area_struct *vma = walk->vma;
> bool locked = !!(vma->vm_flags & VM_LOCKED);
> struct page *page = NULL;
> - bool migration = false;
> + bool migration = false, young = false, dirty = false;
>
> if (pte_present(*pte)) {
> page = vm_normal_page(vma, addr, *pte);
> + young = pte_young(*pte);
> + dirty = pte_dirty(*pte);
> } else if (is_swap_pte(*pte)) {
> swp_entry_t swpent = pte_to_swp_entry(*pte);
>
> @@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> if (!page)
> return;
>
> - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
> - locked, migration);
> + smaps_account(mss, page, false, young, dirty, locked, migration);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
2022-08-03 22:03 [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent Peter Xu
2022-08-04 9:02 ` Vlastimil Babka (SUSE)
2022-08-04 11:01 ` David Hildenbrand
@ 2022-08-04 17:43 ` Yang Shi
2022-08-04 22:40 ` Andrew Morton
3 siblings, 0 replies; 6+ messages in thread
From: Yang Shi @ 2022-08-04 17:43 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, Andrew Morton, Nadav Amit,
David Hildenbrand, Konstantin Khlebnikov
On Wed, Aug 3, 2022 at 3:03 PM Peter Xu <peterx@redhat.com> wrote:
>
> These bits should only be valid when the ptes are present. Introducing two
> booleans for it and set it to false when !pte_present().
>
> Cc: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Fixes: b1d4d9e0cbd0 ("proc/smaps: carefully handle migration entries", 2012-05-31)
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Yang Shi <shy828301@gmail.com>
> ---
> fs/proc/task_mmu.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 9913f3be9fd2..482f91577f8c 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> struct vm_area_struct *vma = walk->vma;
> bool locked = !!(vma->vm_flags & VM_LOCKED);
> struct page *page = NULL;
> - bool migration = false;
> + bool migration = false, young = false, dirty = false;
>
> if (pte_present(*pte)) {
> page = vm_normal_page(vma, addr, *pte);
> + young = pte_young(*pte);
> + dirty = pte_dirty(*pte);
> } else if (is_swap_pte(*pte)) {
> swp_entry_t swpent = pte_to_swp_entry(*pte);
>
> @@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> if (!page)
> return;
>
> - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
> - locked, migration);
> + smaps_account(mss, page, false, young, dirty, locked, migration);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
2022-08-03 22:03 [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent Peter Xu
` (2 preceding siblings ...)
2022-08-04 17:43 ` Yang Shi
@ 2022-08-04 22:40 ` Andrew Morton
2022-08-05 15:52 ` Peter Xu
3 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2022-08-04 22:40 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, linux-mm, Yang Shi, Nadav Amit, David Hildenbrand,
Konstantin Khlebnikov
On Wed, 3 Aug 2022 18:03:29 -0400 Peter Xu <peterx@redhat.com> wrote:
> These bits should only be valid when the ptes are present. Introducing two
> booleans for it and set it to false when !pte_present().
Please (always) describe the user visible runtime effects of the bug
which is being fixed?
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> struct vm_area_struct *vma = walk->vma;
> bool locked = !!(vma->vm_flags & VM_LOCKED);
> struct page *page = NULL;
> - bool migration = false;
> + bool migration = false, young = false, dirty = false;
>
> if (pte_present(*pte)) {
> page = vm_normal_page(vma, addr, *pte);
> + young = pte_young(*pte);
> + dirty = pte_dirty(*pte);
> } else if (is_swap_pte(*pte)) {
> swp_entry_t swpent = pte_to_swp_entry(*pte);
>
> @@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> if (!page)
> return;
>
> - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
> - locked, migration);
> + smaps_account(mss, page, false, young, dirty, locked, migration);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> --
> 2.32.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent
2022-08-04 22:40 ` Andrew Morton
@ 2022-08-05 15:52 ` Peter Xu
0 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2022-08-05 15:52 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-mm, Yang Shi, Nadav Amit, David Hildenbrand,
Konstantin Khlebnikov
On Thu, Aug 04, 2022 at 03:40:50PM -0700, Andrew Morton wrote:
> On Wed, 3 Aug 2022 18:03:29 -0400 Peter Xu <peterx@redhat.com> wrote:
>
> > These bits should only be valid when the ptes are present. Introducing two
> > booleans for it and set it to false when !pte_present().
>
> Please (always) describe the user visible runtime effects of the bug
> which is being fixed?
Will do in the future.
For this specific one I'll add some more details:
The bug is found during code reading and no real world issue reported,
but logically such an error can cause incorrect readings for either smaps
or smaps_rollup output on quite a few fields.
For example, it could cause over-estimate on values like Shared_Dirty,
Private_Dirty, Referenced; or it could also cause under-estimate on
values like LazyFree, Shared_Clean, Private_Clean.
Meanwhile I think I overlooked the pmd handling which seems to have the
same issue but done in another commit.. Will repost soon.
Thanks,
>
>
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -527,10 +527,12 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> > struct vm_area_struct *vma = walk->vma;
> > bool locked = !!(vma->vm_flags & VM_LOCKED);
> > struct page *page = NULL;
> > - bool migration = false;
> > + bool migration = false, young = false, dirty = false;
> >
> > if (pte_present(*pte)) {
> > page = vm_normal_page(vma, addr, *pte);
> > + young = pte_young(*pte);
> > + dirty = pte_dirty(*pte);
> > } else if (is_swap_pte(*pte)) {
> > swp_entry_t swpent = pte_to_swp_entry(*pte);
> >
> > @@ -560,8 +562,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
> > if (!page)
> > return;
> >
> > - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte),
> > - locked, migration);
> > + smaps_account(mss, page, false, young, dirty, locked, migration);
> > }
> >
> > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > --
> > 2.32.0
>
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-05 15:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 22:03 [PATCH] mm/smaps: Don't access young/dirty bit if pte unpresent Peter Xu
2022-08-04 9:02 ` Vlastimil Babka (SUSE)
2022-08-04 11:01 ` David Hildenbrand
2022-08-04 17:43 ` Yang Shi
2022-08-04 22:40 ` Andrew Morton
2022-08-05 15:52 ` Peter Xu
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).