[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] linux-user: Drop unnecessary check in signalfd4 syscall
From: |
Helge Deller |
Subject: |
Re: [PATCH] linux-user: Drop unnecessary check in signalfd4 syscall |
Date: |
Sat, 25 Apr 2020 11:24:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 25.04.20 10:39, Laurent Vivier wrote:
> Le 24/04/2020 à 23:04, Helge Deller a écrit :
>> The signalfd4() syscall takes optional O_NONBLOCK and O_CLOEXEC fcntl
>> flags. If the user gave any other invalid flags, the host syscall will
>> return correct error codes, so simply drop the extra check here.
>>
>> Signed-off-by: Helge Deller <address@hidden>
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 05f03919ff..ebf0d38321 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7176,9 +7176,6 @@ 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)) {
>> - return -TARGET_EINVAL;
>> - }
>> if (!lock_user_struct(VERIFY_READ, target_mask, mask, 1)) {
>> return -TARGET_EFAULT;
>> }
>>
>
> Perhaps we want to trigger the TARGET_EINVAL before the TARGET_EFAULT if
> we have both cases?
>
> But I've checked the kernel, and the kernel does a copy_from_user()
> before checking the flags, but it returns EINVAL rather than EFAULT.
That's not the full picture, since the kernel is not consistent here!
In the compat-case (32bit userspace on 64bit kernel) it returns correctly
EINVAL and EFAULT:
if (sigsetsize != sizeof(compat_sigset_t))
return -EINVAL;
if (get_compat_sigset(&mask, user_mask))
return -EFAULT;
while in the non-compat case it returns EINVAL only:
if (sizemask != sizeof(sigset_t) ||
copy_from_user(&mask, user_mask, sizeof(mask)))
return -EINVAL;
I think the kernel should be fixed here...
> We can remove the flags checking but we should also change TARGET_EFAULT
> by TARGET_EINVAL.
According to the different behaviour of the kernel mentioned above
you won't get it correct either way.
Helge