| Message ID | 20210210061214.GA221322@ls3530.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user: fix O_NONBLOCK in signalfd4() and eventfd2() syscalls | expand |
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. Thanks, Laurent -
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
Le 13/02/2021 à 20:05, Helge Deller a écrit : > 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. > Yes, you're right. Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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; > } > Applied to my linux-user-for-6.0 branch. Thanks, Laurent
Dear qemu-stable, can you please consider adding this patch to the qemu-stable branch. Upstream commit 78721301138114cf37fb179a6cf73a27c1b3a927 Thanks, Helge On 2/13/21 8:16 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; >> } >> > > Applied to my linux-user-for-6.0 branch. > > Thanks, > Laurent >
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; }
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> ---