linux-kernel - Re: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

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]

Message-ID: <2992d40c-ddac-ed5f-ca80-8d3fc08b10e6@collabora.com>
Date:   Fri, 7 Apr 2023 15:50:14 +0500
From:   Muhammad Usama Anjum <usama.anjum@...labora.com>
To:     Michał Mirosław <emmir@...gle.com>
Cc:     Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Peter Xu <peterx@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrei Vagin <avagin@...il.com>,
        Danylo Mocherniuk <mdanylo@...gle.com>,
        Paul Gofman <pgofman@...eweavers.com>,
        Cyrill Gorcunov <gorcunov@...il.com>,
        Mike Rapoport <rppt@...nel.org>, Nadav Amit <namit@...are.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Shuah Khan <shuah@...nel.org>,
        Christian Brauner <brauner@...nel.org>,
        Yang Shi <shy828301@...il.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Yun Zhou <yun.zhou@...driver.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Alex Sierra <alex.sierra@....com>,
        Matthew Wilcox <willy@...radead.org>,
        Pasha Tatashin <pasha.tatashin@...een.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
        Greg KH <gregkh@...uxfoundation.org>, kernel@...labora.com
Subject: Re: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and
 optionally clear info about PTEs

On 4/7/23 3:21 PM, Michał Mirosław wrote:
> On Fri, 7 Apr 2023 at 12:15, Muhammad Usama Anjum
> <usama.anjum@...labora.com> wrote:
>> On 4/7/23 3:04 PM, Michał Mirosław wrote:
>>> On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum
>>> <usama.anjum@...labora.com> wrote:
>>>> On 4/7/23 12:23 PM, Michał Mirosław wrote:
>>>>> On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum
>>>>> <usama.anjum@...labora.com> wrote:
>>>>>> On 4/7/23 1:12 AM, Michał Mirosław wrote:
>>>>>>> On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum
>>>>>>> <usama.anjum@...labora.com> wrote:
>>>>>>> [...]
>>>>>>>> --- a/fs/proc/task_mmu.c
>>>>>>>> +++ b/fs/proc/task_mmu.c
>>>>>>> [...]
>>>>>>>> +static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
>>>>>>>> +                                 unsigned long end, struct mm_walk *walk)
>>>>>>>> +{
>>>>> [...]
>>>>>>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>>>>>> +       ptl = pmd_trans_huge_lock(pmd, vma);
>>>>>>>> +       if (ptl) {
>>>>>>> [...]
>>>>>>>> +               return ret;
>>>>>>>> +       }
>>>>>>>> +process_smaller_pages:
>>>>>>>> +       if (pmd_trans_unstable(pmd))
>>>>>>>> +               return 0;
>>>>>>>
>>>>>>> Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
>>>>>> I'm not entirely sure. But the idea is if THP is unstable, we should
>>>>>> return. As it doesn't seem like after splitting THP can be unstable, we
>>>>>> should not check it. Do you agree with the following?
>>>>>
>>>>> The description of pmd_trans_unstable() [1] seems to indicate that it
>>>>> is needed only after split_huge_pmd().
>>>>>
>>>>> [1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L1394
>>>> Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed
>>>> in normal case when ptl is NULL to rule out the case if pmd is unstable
>>>> before performing operation on normal pages:
>>>>
>>>> ptl = pmd_trans_huge_lock(pmd, vma);
>>>> if (ptl) {
>>>> ...
>>>> }
>>>> if (pmd_trans_unstable(pmd))
>>>>         return 0;
>>>>
>>>> This file has usage examples of pmd_trans_unstable():
>>>>
>>>> https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634
>>>> https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195
>>>> https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543
>>>> https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
>>>>
>>>> So we are good with what we have in this patch.
>>>
>>> Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
>> I'm not sure. I've not done research on it if we need to signal
>> ACTION_AGAIN as this function pagemap_scan_pmd_entry() mimics how
>> pagemap_pmd_range() handles reads to the pagemap file. pagemap_pmd_range()
>> isn't doing anything if pmd is unstable. Hence we also not doing anything.
> 
> Doesn't this mean that if we scan a file-backed vma we would miss
> non-present parts of the mapping in the output?
I'm trying to mimic the same information through ioctl which is attained by
reading the file. We'll only miss the unstable VMA here. I'm don't know
about how often the PMD is unstable and its effects.


> 
> Best Regards
> Michał Mirosław

-- 
BR,
Muhammad Usama Anjum