[LTP] [PATCH v1] security/dirtyc0w_shmem: Fix remaining cases where UFFD_FEATURE_MINOR_SHMEM is absent

[David Hildenbrand]

On 25.11.22 12:20, Martin Doucha wrote:
> Hi,
> 
> On 25. 11. 22 11:52, David Hildenbrand wrote:
>> When UFFD_FEATURE_MINOR_SHMEM is not defined, we still have to make the
>> checkpoint happy, otherwise our parent process will run into a timeout.
>> Further, we have to test if UFFD_FEATURE_MINOR_SHMEM is really returned by
>> the UFFD_API ioctl: if the kernel knows about the feature but doesn't
>> support it, it will be masked off.
>>
>> Reported-by: Martin Doucha <mdoucha@suse.cz>
>> Cc: Petr Vorel <pvorel@suse.cz>
>> Cc: Cyril Hrubis <chrubis@suse.cz>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>    .../dirtyc0w_shmem/dirtyc0w_shmem_child.c        | 16 ++++++++++++----
>>    1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c b/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
>> index cb2e9df0c..c117c6f39 100644
>> --- a/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
>> +++ b/testcases/kernel/security/dirtyc0w_shmem/dirtyc0w_shmem_child.c
>> @@ -24,12 +24,12 @@
>>    #include <linux/userfaultfd.h>
>>    #endif
>>    
>> -#ifdef UFFD_FEATURE_MINOR_SHMEM
>> -
>>    #define TST_NO_DEFAULT_MAIN
>>    #include "tst_test.h"
>>    #include "tst_safe_macros.h"
>>    #include "tst_safe_pthread.h"
>> +
>> +#ifdef UFFD_FEATURE_MINOR_SHMEM
>>    #include "lapi/syscalls.h"
>>    
>>    #define TMP_DIR "tmp_dirtyc0w_shmem"
>> @@ -162,6 +162,10 @@ retry:
>>    			"Could not create userfault file descriptor");
>>    	}
>>    
>> +	if (!(uffdio_api.features & UFFD_FEATURE_MINOR_SHMEM))
>> +		tst_brk(TCONF,
>> +			"System does not have userfaultfd minor fault support for shmem");
>> +
>>    	uffdio_register.range.start = (unsigned long) map;
>>    	uffdio_register.range.len = page_size;
>>    	uffdio_register.mode = UFFDIO_REGISTER_MODE_MINOR;
>> @@ -236,6 +240,10 @@ int main(void)
>>    	return 0;
>>    }
>>    #else /* UFFD_FEATURE_MINOR_SHMEM */
>> -#include "tst_test.h"
>> -TST_TEST_TCONF("System does not have userfaultfd minor fault support for shmem");
>> +int main(void)
>> +{
>> +	tst_reinit();
>> +	TST_CHECKPOINT_WAKE(0);
>> +	tst_brk(TCONF, "System does not have userfaultfd minor fault support for shmem");
>> +}
>>    #endif /* UFFD_FEATURE_MINOR_SHMEM */
> 
> This would work as a workaround but I'd recommend adding the necessary
> structures and constants to include/lapi/userfaultfd.h instead. Then you
> won't need this conditional compilation at all.

That seems to work as well, although I'm still a bit unsure what to 
include in that file, what not, what to strip, ...

I'll use include/lapi/io_uring.h as an orientation but will most 
probably mess it up on the first attempt.

> 
> I also recommend looking at the fuzzy sync library we use for race
> conditions:
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#133-reproducing-race-conditions
> 

I'll look into that as well.

> The original dirtyc0w test was written before this library but using it
> generally makes race condition reproducers faster, simpler and more
> reliable.

Thanks

-- 
Thanks,

David / dhildenb