linux-kernel - Re: [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 27 Jul 2017 13:58:25 +0100
From:   Punit Agrawal <punit.agrawal@....com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Michal Hocko <mhocko@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org, steve.capper@....com,
        will.deacon@....com, catalin.marinas@....com,
        kirill.shutemov@...ux.intel.com
Subject: Re: [PATCH 1/1] mm/hugetlb: Make huge_pte_offset() consistent and document behaviour

Mike Kravetz <mike.kravetz@...cle.com> writes:

> On 07/26/2017 06:34 AM, Punit Agrawal wrote:
>> Michal Hocko <mhocko@...nel.org> writes:
>> 
>>> On Wed 26-07-17 14:33:57, Michal Hocko wrote:
>>>> On Wed 26-07-17 13:11:46, Punit Agrawal wrote:
>>> [...]
>>>>> I've been running tests from mce-test suite and libhugetlbfs for similar
>>>>> changes we did on arm64. There could be assumptions that were not
>>>>> exercised but I'm not sure how to check for all the possible usages.
>>>>>
>>>>> Do you have any other suggestions that can help improve confidence in
>>>>> the patch?
>>>>
>>>> Unfortunatelly I don't. I just know there were many subtle assumptions
>>>> all over the place so I am rather careful to not touch the code unless
>>>> really necessary.
>>>>
>>>> That being said, I am not opposing your patch.
>>>
>>> Let me be more specific. I am not opposing your patch but we should
>>> definitely need more reviewers to have a look. I am not seeing any
>>> immediate problems with it but I do not see a large improvements either
>>> (slightly less nightmare doesn't make me sleep all that well ;)). So I
>>> will leave the decisions to others.
>> 
>> I hear you - I'd definitely appreciate more eyes on the code change and
>> description.
>
> I like the change in semantics for the routine.  Like you, I examined all
> callers of huge_pte_offset() and it appears that they will not be impacted
> by your change.
>
> My only concern is that arch specific versions of huge_pte_offset, may
> not (yet) follow the new semantic.  Someone could potentially introduce
> a new huge_pte_offset call and depend on the new 'documented' semantics.
> Yet, an unmodified arch specific version of huge_pte_offset might have
> different semantics.  I have not reviewed all the arch specific instances
> of the routine to know if this is even possible.  Just curious if you
> examined these, or perhaps you think this is not an issue?

>From checking through the implementations of huge_pte_offset()
architectures, the change shouldn't break anything. (I also cc'd the
posting to linux-arch for architecture maintainers to take more notice).

This is because existing users actively deal with the different returned
values (NULL, huge pte_t*, swap pte_t*) and are not checking explicitly
for pmd or pud.

Guarding against future users is more tricky - it would definitely help
to align all the implementations.