| Message ID | 20210814105131.48814-3-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanups and fixup for memcontrol | expand |
On Sat, Aug 14, 2021 at 06:51:29PM +0800, Miaohe Lin wrote: > If the first pte is equal to poisoned_pfn, i.e. check_hwpoisoned_entry() > return 1, the wrong ptep - 1 would be passed to pte_unmap_unlock(). > > Fixes: ad9c59c24095 ("mm,hwpoison: send SIGBUS with error virutal address") > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> I agree with the change itself, so Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> One question is that according to "grep -r pte_unmap_unlock ." command over whole kernel source code, pte_unmap_unlock() is called with "ptep - 1" in some places. I think that none of them seems to have "break in for loop" in locked period, so the same problem does not occur there. But I'm still not sure why some place call with "ptep - 1" and the others call with pte returned by pte_offset_map_lock().
On 2021/8/17 15:29, HORIGUCHI NAOYA(堀口 直也) wrote: > On Sat, Aug 14, 2021 at 06:51:29PM +0800, Miaohe Lin wrote: >> If the first pte is equal to poisoned_pfn, i.e. check_hwpoisoned_entry() >> return 1, the wrong ptep - 1 would be passed to pte_unmap_unlock(). >> >> Fixes: ad9c59c24095 ("mm,hwpoison: send SIGBUS with error virutal address") >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > I agree with the change itself, so > > Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > Many thanks for your review and Acked-by tag! > One question is that according to "grep -r pte_unmap_unlock ." command over > whole kernel source code, pte_unmap_unlock() is called with "ptep - 1" in some places. > I think that none of them seems to have "break in for loop" in locked period, > so the same problem does not occur there. But I'm still not sure why some place > call with "ptep - 1" and the others call with pte returned by pte_offset_map_lock(). IMO pte_unmap_unlock() works as long as the passed in pte belongs to the same page returned from pte_offset_map_lock(). I have fixed some similar place where pte_unmap_unlock() is called with wrong "ptep - 1" when I was learning the related mm code. >
On Tue, Aug 17, 2021 at 04:24:43PM +0800, Miaohe Lin wrote: > On 2021/8/17 15:29, HORIGUCHI NAOYA(堀口 直也) wrote: ... > > One question is that according to "grep -r pte_unmap_unlock ." command over > > whole kernel source code, pte_unmap_unlock() is called with "ptep - 1" in some places. > > I think that none of them seems to have "break in for loop" in locked period, > > so the same problem does not occur there. But I'm still not sure why some place > > call with "ptep - 1" and the others call with pte returned by pte_offset_map_lock(). > > IMO pte_unmap_unlock() works as long as the passed in pte belongs to the same page returned > from pte_offset_map_lock(). I have fixed some similar place where pte_unmap_unlock() is called > with wrong "ptep - 1" when I was learning the related mm code. Great, thanks for clarification. - Naoya
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 052ec9ee7cf6..54f61133bf60 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -632,7 +632,7 @@ static int hwpoison_pte_range(pmd_t *pmdp, unsigned long addr, { struct hwp_walk *hwp = (struct hwp_walk *)walk->private; int ret = 0; - pte_t *ptep; + pte_t *ptep, *mapped_pte; spinlock_t *ptl; ptl = pmd_trans_huge_lock(pmdp, walk->vma); @@ -645,14 +645,15 @@ static int hwpoison_pte_range(pmd_t *pmdp, unsigned long addr, if (pmd_trans_unstable(pmdp)) goto out; - ptep = pte_offset_map_lock(walk->vma->vm_mm, pmdp, addr, &ptl); + mapped_pte = ptep = pte_offset_map_lock(walk->vma->vm_mm, pmdp, + addr, &ptl); for (; addr != end; ptep++, addr += PAGE_SIZE) { ret = check_hwpoisoned_entry(*ptep, addr, PAGE_SHIFT, hwp->pfn, &hwp->tk); if (ret == 1) break; } - pte_unmap_unlock(ptep - 1, ptl); + pte_unmap_unlock(mapped_pte, ptl); out: cond_resched(); return ret;
If the first pte is equal to poisoned_pfn, i.e. check_hwpoisoned_entry() return 1, the wrong ptep - 1 would be passed to pte_unmap_unlock(). Fixes: ad9c59c24095 ("mm,hwpoison: send SIGBUS with error virutal address") Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/memory-failure.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)