linux-kernel - Re: selftests/vm madv

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:   Fri, 15 Oct 2021 18:19:56 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: selftests/vm madv_populate.c test

On 15.10.21 18:15, David Hildenbrand wrote:
> On 15.10.21 18:06, David Hildenbrand wrote:
>> On 15.10.21 17:47, David Hildenbrand wrote:
>>> On 15.10.21 17:45, Shuah Khan wrote:
>>>> On 9/18/21 1:41 AM, David Hildenbrand wrote:
>>>>> On 18.09.21 00:45, Shuah Khan wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> I am running into the following warning when try to build this test:
>>>>>>
>>>>>> madv_populate.c:334:2: warning: #warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition" [-Wcpp]
>>>>>>     334 | #warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
>>>>>>         |  ^~~~~~~
>>>>>>
>>>>>>
>>>>>> I see that the following handling is in place. However there is no
>>>>>> other information to explain why the check is necessary.
>>>>>>
>>>>>> #if defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE)
>>>>>>
>>>>>> #else /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */
>>>>>>
>>>>>> #warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
>>>>>>
>>>>>> I do see these defined in:
>>>>>>
>>>>>> include/uapi/asm-generic/mman-common.h:#define MADV_POPULATE_READ       22
>>>>>> include/uapi/asm-generic/mman-common.h:#define MADV_POPULATE_WRITE      23
>>>>>>
>>>>>> Is this the case of missing include from madv_populate.c?
>>>>>
>>>>> Hi Shuan,
>>>>>
>>>>> note that we're including "#include <sys/mman.h>", which in my
>>>>> understanding maps to the version installed on your system instead
>>>>> of the one in our build environment.ing.
>>>>>
>>>>> So as soon as you have a proper kernel + the proper headers installed
>>>>> and try to build, it would pick up MADV_POPULATE_READ and
>>>>> MADV_POPULATE_WRITE from the updated headers. That makes sense: you
>>>>> annot run any MADV_POPULATE_READ/MADV_POPULATE_WRITE tests on a kernel
>>>>> that doesn't support it.
>>>>>
>>>>> See vm/userfaultfd.c where we do something similar.
>>>>>
>>>>
>>>> Kselftest is for testing the kernel with kernel headers. That is the
>>>> reason why there is the dependency on header install.
>>>>
>>>>>
>>>>> As soon as we have a proper environment, it seems to work just fine:
>>>>>
>>>>> Linux vm-0 5.15.0-0.rc1.20210915git3ca706c189db.13.fc36.x86_64 #1 SMP Thu Sep 16 11:32:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
>>>>> [root@...0 linux]# cat /etc/redhat-release
>>>>> Fedora release 36 (Rawhide)
>>>>
>>>> This is a distro release. We don't want to have dependency on headers
>>>> from the distro to run selftests. Hope this makes sense.
>>>>
>>>> I still see this on my test system running Linux 5.15-rc5.
>>>
>>> Did you also install Linux headers? I assume no, correct?
>>>
>>
>> What happens in your environment when compiling and running the
>> memfd_secret test?
>>
>> If assume you'll see a "skip" when executing, because it might also
>> refer to the local version of linux headers and although it builds, it
>> really cannot build something "functional". It just doesn't add a
>> "#warning" to make that obvious.
>>
> 
> The following works but looks extremely hackish.
> 
> diff --git a/tools/testing/selftests/vm/madv_populate.c
> b/tools/testing/selftests/vm/madv_populate.c
> index b959e4ebdad4..ab26163db540 100644
> --- a/tools/testing/selftests/vm/madv_populate.c
> +++ b/tools/testing/selftests/vm/madv_populate.c
> @@ -14,12 +14,11 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include "../../../../usr/include/linux/mman.h"
>  #include <sys/mman.h>
> 
>  #include "../kselftest.h"
> 
> -#if defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE)
> -
>  /*
>   * For now, we're using 2 MiB of private anonymous memory for all tests.
>   */
> @@ -328,15 +327,3 @@ int main(int argc, char **argv)
>                                    err, ksft_test_num());
>         return ksft_exit_pass();
>  }
> -
> -#else /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */
> -
> -#warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
> -
> -int main(int argc, char **argv)
> -{
> -       ksft_print_header();
> -       ksft_exit_skip("MADV_POPULATE_READ or MADV_POPULATE_WRITE not
> defined\n");
> -}
> -
> -#endif /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */
> 
> 
> There has to be some clean way to achieve the same.
> 

Sorry for the spam,

diff --git a/tools/testing/selftests/vm/Makefile
b/tools/testing/selftests/vm/Makefile
index d9605bd10f2d..ce198b329ff5 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -23,7 +23,7 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e
's/aarch64.*/arm64/' -e 's/ppc64.*/p
 # LDLIBS.
 MAKEFLAGS += --no-builtin-rules

-CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
+CFLAGS = -Wall -idirafter ../../../../usr/include $(EXTRA_CFLAGS)
 LDLIBS = -lrt -lpthread
 TEST_GEN_FILES = compaction_test
 TEST_GEN_FILES += gup_test


Seems to set the right include path priority.


-- 
Thanks,

David / dhildenb