linux-kernel - Re: [PATCH v2 1/1] selftests: vm: add process

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: <7f0fd407-18f5-2718-40b5-b16804163197@linuxfoundation.org>
Date:   Mon, 16 May 2022 17:28:01 -0600
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        David Rientjes <rientjes@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Roman Gushchin <guro@...com>, Minchan Kim <minchan@...nel.org>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Christian Brauner <brauner@...nel.org>,
        Christoph Hellwig <hch@...radead.org>,
        Oleg Nesterov <oleg@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Jann Horn <jannh@...gle.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Peter Xu <peterx@...hat.com>,
        John Hubbard <jhubbard@...dia.com>, shuah@...nel.org,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>, linux-kselftest@...r.kernel.org,
        kernel-team <kernel-team@...roid.com>,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2 1/1] selftests: vm: add process_mrelease tests

On 5/16/22 2:47 PM, Suren Baghdasaryan wrote:
> On Mon, May 16, 2022 at 1:29 PM Shuah Khan <skhan@...uxfoundation.org> wrote:
>>
>> On 5/16/22 1:55 AM, Suren Baghdasaryan wrote:
>>> Introduce process_mrelease syscall sanity tests which include tests
>>> which expect to fail:
>>> - process_mrelease with invalid pidfd and flags inputs
>>> - process_mrelease on a live process with no pending signals
>>> and valid process_mrelease usage which is expected to succeed.
>>> Because process_mrelease has to be used against a process with a pending
>>> SIGKILL, it's possible that the process exits before process_mrelease
>>> gets called. In such cases we retry the test with a victim that allocates
>>> twice more memory up to 1GB. This would require the victim process to
>>> spend more time during exit and process_mrelease has a better chance of
>>> catching the process before it exits and succeeding.
>>>
>>> On success the test reports the amount of memory the child had to
>>> allocate for reaping to succeed. Sample output:
>>>       Success reaping a child with 1MB of memory allocations
>>>
>>> On failure the test reports the failure. Sample outputs:
>>>       All process_mrelease attempts failed!
>>>       process_mrelease: Invalid argument
>>>
>>
>> Nit: Please format this better - include actual example output from the
>> command and how to run the test examples.
> 
> Hmm... Those are the actual outputs from the command and it does not
> take any input arguments. Do you mean smth like this:
> 
> $ mrelease_test
> Success reaping a child with 1MB of memory allocations
> 
> $ mrelease_test
> All process_mrelease attempts failed!
> 
> $ mrelease_test
> process_mrelease: Invalid argument
> 
> ?

This looks good.

> 
>>
>>> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
>>> ---
>>>    tools/testing/selftests/vm/.gitignore      |   1 +
>>>    tools/testing/selftests/vm/Makefile        |   1 +
>>>    tools/testing/selftests/vm/mrelease_test.c | 214 +++++++++++++++++++++
>>>    tools/testing/selftests/vm/run_vmtests.sh  |  16 ++
>>>    4 files changed, 232 insertions(+)
>>>    create mode 100644 tools/testing/selftests/vm/mrelease_test.c
>>>

[snip]

>>
>> Okay these above 3 routines are called once. I am not seeing any point
>> in making these separate routines. I made the same comment on v1.
> 
> I must have misunderstood your previous comment. Will change.
> 

Thank you.

>>

>>
>> Now the above code can be a separate function which will make it readable.
> 
> Ack.
> 
>>

>>> +
>>
>> Why do you need these ifdefs - syscall will return ENOSYS and you can
>> key off that. Please take a look at other usages of syscall in the
>> repo.
> 
> The issue is that I need to provide the syscall number when calling
> syscall() (in my case __NR_pidfd_open and __NR_process_mrelease) and
> if that number is not defined in the userspace headers on a given
> system then what should I pass instead?
> When implementing this I followed the examples of
> https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/memfd_secret.c#L30
> and https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/userfaultfd.c#L65.
> My original implementation was modeled after this approach:
> https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/mlock2.h#L15.
> If none of these are correct, could you please point me to the example
> you want me to follow?
> 

kselftests include kernel headers. As long as these syscalls are
defined in the kernel headers, the test will build.

Looks it is defined in include/uapi/asm-generic/unistd.h

You can assume it is defined and then if we find architectures that
don't, you can follow what tools/testing/selftests/pidfd/pidfd.h
does.

This way the test can simply call syscall and handle ENOSYS.

thanks,
-- Shuah