Received-SPF: pass (google.com: domain of linux-kernel+bounces-167386-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <0afb3887-f72b-42d3-8809-e97353fae031@arm.com>
Date: Fri, 3 May 2024 09:30:23 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v10 3/5] selftest mm/mseal memory sealing
Content-Language: en-GB
To: Jeff Xu <jeffxu@chromium.org>
Cc: akpm@linux-foundation.org, keescook@chromium.org, jannh@google.com,
 sroettger@google.com, willy@infradead.org, gregkh@linuxfoundation.org,
 torvalds@linux-foundation.org, usama.anjum@collabora.com, corbet@lwn.net,
 Liam.Howlett@oracle.com, surenb@google.com, merimus@google.com,
 rdunlap@infradead.org, jeffxu@google.com, jorgelo@chromium.org,
 groeck@chromium.org, linux-kernel@vger.kernel.org,
 linux-kselftest@vger.kernel.org, linux-mm@kvack.org,
 pedro.falcato@gmail.com, dave.hansen@intel.com,
 linux-hardening@vger.kernel.org, deraadt@openbsd.org
References: <20240415163527.626541-1-jeffxu@chromium.org>
 <20240415163527.626541-4-jeffxu@chromium.org>
 <f797fbde-ffb7-44b0-8af6-4ed2ec47eac1@arm.com>
 <CABi2SkXmGR41o8LwM=oD-PCZWvcc5zOie65wvuk5zsAQPymmRA@mail.gmail.com>
From: Ryan Roberts <ryan.roberts@arm.com>
In-Reply-To: <CABi2SkXmGR41o8LwM=oD-PCZWvcc5zOie65wvuk5zsAQPymmRA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 02/05/2024 23:39, Jeff Xu wrote:
> On Thu, May 2, 2024 at 4:24 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 15/04/2024 17:35, jeffxu@chromium.org wrote:
>>> From: Jeff Xu <jeffxu@chromium.org>
>>>
>>> selftest for memory sealing change in mmap() and mseal().
>>>
>>> Signed-off-by: Jeff Xu <jeffxu@chromium.org>
>>> ---
>>>  tools/testing/selftests/mm/.gitignore   |    1 +
>>>  tools/testing/selftests/mm/Makefile     |    1 +
>>>  tools/testing/selftests/mm/mseal_test.c | 1836 +++++++++++++++++++++++
>>>  3 files changed, 1838 insertions(+)
>>>  create mode 100644 tools/testing/selftests/mm/mseal_test.c
>>>
>>> diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
>>> index d26e962f2ac4..98eaa4590f11 100644
>>> --- a/tools/testing/selftests/mm/.gitignore
>>> +++ b/tools/testing/selftests/mm/.gitignore
>>> @@ -47,3 +47,4 @@ mkdirty
>>>  va_high_addr_switch
>>>  hugetlb_fault_after_madv
>>>  hugetlb_madv_vs_map
>>> +mseal_test
>>> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
>>> index eb5f39a2668b..95d10fe1b3c1 100644
>>> --- a/tools/testing/selftests/mm/Makefile
>>> +++ b/tools/testing/selftests/mm/Makefile
>>> @@ -59,6 +59,7 @@ TEST_GEN_FILES += mlock2-tests
>>>  TEST_GEN_FILES += mrelease_test
>>>  TEST_GEN_FILES += mremap_dontunmap
>>>  TEST_GEN_FILES += mremap_test
>>> +TEST_GEN_FILES += mseal_test
>>>  TEST_GEN_FILES += on-fault-limit
>>>  TEST_GEN_FILES += pagemap_ioctl
>>>  TEST_GEN_FILES += thuge-gen
>>> diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
>>> new file mode 100644
>>> index 000000000000..06c780d1d8e5
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/mm/mseal_test.c
>>> @@ -0,0 +1,1836 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +#define _GNU_SOURCE
>>> +#include <sys/mman.h>
>>
>> I'm afraid this is causing a build error on our CI, and as a result we are not
>> running any mm selftests currently.
>>
>> The error is here:
>>
>>   CC       mseal_test
>> mseal_test.c: In function ‘test_seal_mremap_move_dontunmap’:
>> mseal_test.c:1469:50: error: ‘MREMAP_DONTUNMAP’ undeclared (first use in this
>> function)
>>  1469 |  ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP, 0);
>>       |                                                  ^~~~~~~~~~~~~~~~
>> mseal_test.c:1469:50: note: each undeclared identifier is reported only once for
>> each function it appears in
>> mseal_test.c: In function ‘test_seal_mremap_move_dontunmap_anyaddr’:
>> mseal_test.c:1501:50: error: ‘MREMAP_DONTUNMAP’ undeclared (first use in this
>> function)
>>  1501 |  ret2 = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_DONTUNMAP,
>>       |                                                  ^~~~~~~~~~~~~~~~
>>
>>
>> And I think the reason is due to our CI's toolchain's sys/mman.h not including
>> linux/mman.h where MREMAP_DONTUNMAP is defined.
>>
>> I think the fix is to explicitly #include <linux/mman.h>, as a number of other
>> mm selftests do.
>>
> When I tried to build with aarch64-linux-gnu-gcc, this passed.
> 
> aarch64-linux-gnu-gcc -I ../../../../usr/include   -DDEBUG -O3
> -DDEBUG -O3 mseal_test.c -o  mseal_test -lm -Wall

Its the same on my local system; I'm told our CI is using GCC 10, which I
suspect makes the difference.

> 
> I don't have the exact environment to repro the issue and verify the fix.
> I will send a patch with  the linux/mman.h.
> 
> I will probably need some help to verify the fix on arm build, Ryan,
> could you help with this ?

I'll pass this on to our CI folks, and hopefully get confirmation shortly.

> 
> Thanks
> -Jeff