Re: [PATCH] linux-user: fix O_NONBLOCK in signalfd4() and eventfd2() syscalls

[Helge Deller <deller@gmx.de>]

On 2/13/21 5:38 PM, Laurent Vivier wrote:
> Le 10/02/2021 à 07:12, Helge Deller a écrit :
>> On the hppa target userspace binaries may call signalfd4() and
>> eventfd2() with an old TARGET_O_NONBLOCK value of 000200004 instead of
>> 000200000 for the "mask" syscall parameter, in which case the current
>> emulation doesn't handle the translation to the native O_NONBLOCK value
>> correctly.
>>
>> The 0x04 bit is not masked out before the new O_NONBLOCK bit is set and
>> as such when calling the native syscall errors out with EINVAL.
>>
>> Fix this by introducing TARGET_O_NONBLOCK_MASK which is used to mask off
>> all possible bits. This define defaults to TARGET_O_NONBLOCK when not
>> defined otherwise, so for all other targets the implementation will
>> behave as before.
>>
>> This patch needs to be applied on top of my previous two patches.
>>
>> Bug was found and patch was verified by using qemu-hppa as debian buildd
>> server on x86_64.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> ---
>>
>> diff --git a/linux-user/hppa/target_fcntl.h b/linux-user/hppa/target_fcntl.h
>> index 08e3a4fcb0..4eb0ec98e2 100644
>> --- a/linux-user/hppa/target_fcntl.h
>> +++ b/linux-user/hppa/target_fcntl.h
>> @@ -9,6 +9,7 @@
>>   #define HPPA_TARGET_FCNTL_H
>>
>>   #define TARGET_O_NONBLOCK    000200000
>> +#define TARGET_O_NONBLOCK_MASK 000200004 /* includes old HP-UX NDELAY flag */
>>   #define TARGET_O_APPEND      000000010
>>   #define TARGET_O_CREAT       000000400 /* not fcntl */
>>   #define TARGET_O_EXCL        000002000 /* not fcntl */
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 27adee908e..3031aa342f 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -273,6 +273,11 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5,	\
>>   #define TARGET_NR__llseek TARGET_NR_llseek
>>   #endif
>>
>> +/* some platforms need to mask more bits than just TARGET_O_NONBLOCK */
>> +#ifndef TARGET_O_NONBLOCK_MASK
>> +#define TARGET_O_NONBLOCK_MASK TARGET_O_NONBLOCK
>> +#endif
>> +
>>   #define __NR_sys_gettid __NR_gettid
>>   _syscall0(int, sys_gettid)
>>
>> @@ -7719,7 +7724,7 @@ static abi_long do_signalfd4(int fd, abi_long mask, int flags)
>>       sigset_t host_mask;
>>       abi_long ret;
>>
>> -    if (flags & ~(TARGET_O_NONBLOCK | TARGET_O_CLOEXEC)) {
>> +    if (flags & ~(TARGET_O_NONBLOCK_MASK | TARGET_O_CLOEXEC)) {
>>           return -TARGET_EINVAL;
>>       }
>>       if (!lock_user_struct(VERIFY_READ, target_mask, mask, 1)) {
>> @@ -12508,7 +12513,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>>   #if defined(TARGET_NR_eventfd2)
>>       case TARGET_NR_eventfd2:
>>       {
>> -        int host_flags = arg2 & (~(TARGET_O_NONBLOCK | TARGET_O_CLOEXEC));
>> +        int host_flags = arg2 & (~(TARGET_O_NONBLOCK_MASK | TARGET_O_CLOEXEC));
>>           if (arg2 & TARGET_O_NONBLOCK) {
>>               host_flags |= O_NONBLOCK;
>>           }
>>
>
> You might also update fcntl_flags_tbl[], the first column is the mask.

No, I intentionally did not updated the fcntl_flags_tbl[] entry.
The check in target_to_host_bitmask() is:
         if ((target_mask & btp->target_mask) == btp->target_bits)
and the table entry is:
  { TARGET_O_NONBLOCK,  TARGET_O_NONBLOCK,  O_NONBLOCK,  O_NONBLOCK,  },

Application may hand in 000200000 *or* 000200004 as value.
So checking (value & 000200000) == 000200000 is correct for both cases.

Helge