linux-kernel - Re: [PATCH 1/2] selftests/memfd/memfd

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] [day] [month] [year] [list]

Date:   Wed, 4 Apr 2018 09:54:03 -0600
From:   Shuah Khan <shuah@...nel.org>
To:     Mike Kravetz <mike.kravetz@...cle.com>,
        Anders Roxell <anders.roxell@...aro.org>
Cc:     linux-kselftest@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        David Herrmann <dh.herrmann@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH 1/2] selftests/memfd/memfd_test.c: fix implicit
 declaration

On 04/04/2018 09:32 AM, Mike Kravetz wrote:
> On 04/04/2018 04:36 AM, Anders Roxell wrote:
>> On 14 March 2018 at 02:09, Mike Kravetz <mike.kravetz@...cle.com> wrote:
>>> On 03/13/2018 04:42 AM, Anders Roxell wrote:
>>>> gcc warns about implicit declaration.
>>>>
>>>> gcc -D_FILE_OFFSET_BITS=64 -I../../../../include/uapi/
>>>>     -I../../../../include/ -I../../../../usr/include/
>>>>     memfd_test.c common.o  -o memfd_test
>>>> memfd_test.c: In function ‘mfd_assert_get_seals’:
>>>> memfd_test.c:74:6: warning: implicit declaration of function ‘fcntl’
>>>>     [-Wimplicit-function-declaration]
>>>>   r = fcntl(fd, F_GET_SEALS);
>>>>       ^~~~~
>>>> memfd_test.c: In function ‘mfd_assert_open’:
>>>> memfd_test.c:197:6: warning: implicit declaration of function ‘open’
>>>>     [-Wimplicit-function-declaration]
>>>>   r = open(buf, flags, mode);
>>>>       ^~~~
>>>> memfd_test.c: In function ‘mfd_assert_write’:
>>>> memfd_test.c:328:6: warning: implicit declaration of function ‘fallocate’
>>>>     [-Wimplicit-function-declaration]
>>>>   r = fallocate(fd,
>>>>       ^~~~~~~~~
>>>>
>>>> In the current code, we include the headers that the functions want
>>>> according to the man pages, and we add some defines that will be used if
>>>> they isn't found in glibc.  The defines was added into the kernel source
>>>> in kernel >= 3.16 and glibc requires kernel header files >= 3.2>>>>
>>>> Fixes: 4f5ce5e8d7e2 ("selftests: add memfd_create() + sealing tests")
>>>> Signed-off-by: Anders Roxell <anders.roxell@...aro.org>
>>>> ---
>>>>  tools/testing/selftests/memfd/memfd_test.c | 25 ++++++++++++++++++++++++-
>>>>  1 file changed, 24 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
>>>> index 10baa1652fc2..0dbeb29c094c 100644
>>>> --- a/tools/testing/selftests/memfd/memfd_test.c
>>>> +++ b/tools/testing/selftests/memfd/memfd_test.c
>>>> @@ -6,7 +6,6 @@
>>>>  #include <inttypes.h>
>>>>  #include <limits.h>
>>>>  #include <linux/falloc.h>
>>>> -#include <linux/fcntl.h>

We don't want to change this to fix the warnings. This will defeat
the purpose of testing the kernel including the headers.

>>>>  #include <linux/memfd.h>
>>>>  #include <sched.h>
>>>>  #include <stdio.h>
>>>> @@ -14,13 +13,37 @@
>>>>  #include <signal.h>
>>>>  #include <string.h>
>>>>  #include <sys/mman.h>
>>>> +#include <sys/types.h>
>>>>  #include <sys/stat.h>
>>>>  #include <sys/syscall.h>
>>>>  #include <sys/wait.h>
>>>> +#include <fcntl.h>
>>>>  #include <unistd.h>
>>>
>>> I suspect there is some guiding philosophy for selftests that I am
>>> unfamiliar with.  However, it seems that tests should use as much
>>> of the header files in the current kernel source tree as possible.
>>> This change removes the include of a header in the current source
>>> tree <linux/fcntl.h>.  It replaces that with the header <fcntl.h>
>>> from the host system (and some other changes).
>>>
>>> To me, this seems like step in the wrong direction.  But, I could
>>> be totally wrong and perhaps self tests should primarily target the
>>> host system header files.
>>
>> So in a way I agree with you. However, what was the design decisions
>> internal kernel headers vs. headers from Host System ?
>> For me it isn't clear how/when we use them, its a mix today is it not?

Internal headers are the primary focus to be able to find regressions.

> 
> I am unsure as well.  Was hoping someone who was more involved in the
> development/design philosophy of self tests would chime in and comment.
> 
> My thought that the tests should use more of the associated kernel header
> files is based on the fact that those header files 'should' be more
> aligned with the tests.  The host system header files could be very
> different than those of the running kernel we are testing.

Right. The goal for these tests is to verify the kernel including the
headers. So for best results, the tests should be built with the headers
in the kernel not the ones that are installed on the system they are being
built on.

thanks,
-- Shuah