Re: [PATCH v2] Test errno setup

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 09/03/2017 19:29, Yury Norov wrote:
> On Thu, Mar 09, 2017 at 05:30:03PM +0100, Adhemerval Zanella wrote:
>>> Regarding mlock-syscall:
>>> If the compat mlock syscall is used, it returns 12 (ENOMEM).
>>> This is also observable if you compile and run the testcase with -m32 on a x86_64 system.
>>>
>>>
>>> I've compiled and run posix/test-errno.c on my s390x system and
>>> get the following error:
>>> FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file descriptor)
>>>
>>> sl=0xfdfa9170 before setsockopt syscall.
>>> The test succeeds if I sl is initialized to zero.
>>>
>>
>> POSIX [1] states that mlock should may fail with EINVAL only if
>> the addr argument is not a multiple of {PAGESIZE}.  Linux does
>> not return this issue, since it aligns the resulting address
>> to pagesize:
>>
>> * mm/mlock.c:
>>
>> 666 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
>> 667 {
>> [...]
>> 676 
>> 677         len = PAGE_ALIGN(len + (offset_in_page(start)));
>> 678         start &= PAGE_MASK;
>>
>> EINVAL is only returned on 'apply_vma_lock_flags':
>>
>> 578 static int apply_vma_lock_flags(unsigned long start, size_t len,
>> 579                                 vm_flags_t flags)
>> 580 {
>> [...]
>> 587         end = start + len;
>> 588         if (end < start)
>> 589                 return -EINVAL;
>>
>> But if you runs 32 binaries on 64 bits kernel overflow won't happen.
>> It is documented in man pages, so I think from kernel side it should
>> be consistent for 32 bit binaries on 64 bit kernel.
> 
> I also observed this fail in aarch64/ilp32, but I decided that this
> is the ilp32 problem while it's compat one, as you discovered here.
> 
> The test is not intended to check each and every syscall, as it is
> mentioned in comment. For example, all syscalls that may fail with
> EFAULT are not tested because they may both return the EFAULT or
> actually terminate the application with segfault. This is the
> similar case for me. I think that it's better to keep the test simple,
> and just drop the mlock test, and not introduce another macro.
> 
> Though, the patch that handles the issue is already there, and I don't
> object if you submit it.

I do not have a strong opinion here, but since adjusting to check for
both results is pretty much straightforward I think it is worth to just
add it and comment why it requires to check for both results.  I will
send a patch upstream shortly.