lkml.org 
[lkml]   [2019]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH STABLE 4.9 1/1] mm, gup: add missing refcount overflow checks on x86 and s390
From
Date
On 12/3/19 1:22 PM, Ben Hutchings wrote:
>> + || !page_cache_get_speculative(head)))
>> return 0;
>> if (unlikely(pte_val(pte) != pte_val(*ptep))) {
>> put_page(head);
> [...]
>> --- a/arch/x86/mm/gup.c
>> +++ b/arch/x86/mm/gup.c
>> @@ -202,10 +202,12 @@ static int __gup_device_huge_pmd(pmd_t pmd, unsigned long addr,
>> undo_dev_pagemap(nr, nr_start, pages);
>> return 0;
>> }
>> + if (unlikely(!try_get_page(page))) {
>> + put_dev_pagemap(pgmap);
>> + return 0;
>> + }
>> SetPageReferenced(page);
>> pages[*nr] = page;
>> - get_page(page);
>> - put_dev_pagemap(pgmap);
>
> This leaks a pgmap reference on success!

Good catch, deleted one line too many!

>> (*nr)++;
>> pfn++;
>> } while (addr += PAGE_SIZE, addr != end);
>> @@ -230,6 +232,8 @@ static noinline int gup_huge_pmd(pmd_t pmd, unsigned long addr,
>>
>> refs = 0;
>> head = pmd_page(pmd);
>> + if (WARN_ON_ONCE(page_ref_count(head) <= 0))
>
> Why <= 0, given we use < 0 elsewhere?

The code uses get_head_page_multiple() which boils down to atomic_add
and not add_unless_zero(), so it assumes a pre-existing pin that must
not go away or it's a bug (one that I've been hunting recently in this
area). The check makes it explicit.

>
>> + return 0;
>> page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
>> do {
>> VM_BUG_ON_PAGE(compound_head(page) != head, page);
>> @@ -289,6 +293,8 @@ static noinline int gup_huge_pud(pud_t pud, unsigned long addr,
>>
>> refs = 0;
>> head = pud_page(pud);
>> + if (WARN_ON_ONCE(page_ref_count(head) <= 0))
>
> Same question here.

Same as above.

> Ben.
>
>> + return 0;
>> page = head + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
>> do {
>> VM_BUG_ON_PAGE(compound_head(page) != head, page);

\
 
 \ /
  Last update: 2019-12-03 13:48    [W:0.044 / U:1.988 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site