linux-kernel - Re: [PATCH v4] mm: fix is_pinnable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 May 2022 19:55:25 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Minchan Kim <minchan@...nel.org>
CC:     Jason Gunthorpe <jgg@...pe.ca>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-mm <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        John Dias <joaodias@...gle.com>,
        "David Hildenbrand" <david@...hat.com>
Subject: Re: [PATCH v4] mm: fix is_pinnable_page against on cma page

On 5/23/22 09:33, Minchan Kim wrote:
...
>> So then:
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 0e42038382c1..b404f87e2682 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -482,7 +482,12 @@ unsigned long __get_pfnblock_flags_mask(const struct page *page,
>>          word_bitidx = bitidx / BITS_PER_LONG;
>>          bitidx &= (BITS_PER_LONG-1);
>>
>> -       word = bitmap[word_bitidx];
>> +       /*
>> +        * This races, without locks, with set_pageblock_migratetype(). Ensure
>                                                   
>                                               set_pfnblock_flags_mask would be better?
>                            
>> +        * a consistent (non-tearing) read of the memory array, so that results,
> 
> Thanks for proceeding and suggestion, John.
> 
> IIUC, the load tearing wouldn't be an issue since [1] fixed the issue.

Did it? [1] fixed something, but I'm not sure we can claim that that
code is now safe against tearing in all possible cases, especially given
the recent discussion here. Specifically, having this code do a read,
then follow that up with calculations, seems correct. Anything else is
sketchy...

> 
> The concern in our dicussion was aggressive compiler(e.g., LTO) or code refactoring
> to make the code inline in *future* could potentially cause forcing refetching(i.e.,
> re-read) tie bitmap[word_bitidx].
> 
> If so, shouldn't the comment be the one you helped before?

Well, maybe updated to something like this?

/*
  * This races, without locks, with set_pageblock_migratetype(). Ensure
  * a consistent (non-tearing) read of the memory array, so that results,
  * even though racy, are not corrupted--even if this function is
  * refactored and/or inlined.
  */



thanks,
-- 
John Hubbard
NVIDIA