Messages in this thread | | | Subject | Re: [PATCH] mm: fix potential pte_unmap_unlock pte error | From | Shijie Luo <> | Date | Thu, 15 Oct 2020 21:19:06 +0800 |
| |
On 2020/10/15 20:58, osalvador@suse.de wrote: > On 2020-10-15 14:15, Shijie Luo wrote: >> When flags don't have MPOL_MF_MOVE or MPOL_MF_MOVE_ALL bits, code breaks >> and passing origin pte - 1 to pte_unmap_unlock seems like not a good >> idea. >> >> Signed-off-by: Shijie Luo <luoshijie1@huawei.com> >> Signed-off-by: linmiaohe <linmiaohe@huawei.com> >> --- >> mm/mempolicy.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/mm/mempolicy.c b/mm/mempolicy.c >> index 3fde772ef5ef..01f088630d1d 100644 >> --- a/mm/mempolicy.c >> +++ b/mm/mempolicy.c >> @@ -571,7 +571,11 @@ static int queue_pages_pte_range(pmd_t *pmd, >> unsigned long addr, >> } else >> break; >> } >> - pte_unmap_unlock(pte - 1, ptl); >> + >> + if (addr >= end) >> + pte = pte - 1; >> + >> + pte_unmap_unlock(pte, ptl); > > But this is still wrong, isn't it? > Unless I am missing something, this is "only" important under > CONFIG_HIGHPTE. > > We have: > > pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl); > > which under CONFIG_HIGHPTE does a kmap_atomoc. > > Now, we either break the loop in the first pass because of > !(MPOL_MF_MOVE | MPOL_MF_MOVE_ALL), > or we keep incrementing pte by every pass. > Either way is wrong, because the pointer kunmap_atomic gets will not > be the same (since we incremented pte). > > Or is the loop meant to be running only once, so pte - 1 will bring us > back to the original pte? > > .
Thanks for your reply, if we break the loop in the first pass, the pte pointer will not be incremented,
pte - 1 equals original pte - 1, because we only increase pte pointer when not break the loop.
| |