Re: [PATCH] linux-user: Drop unnecessary check in dup3 syscall

[Eric Blake <eblake@redhat.com>]

On 4/24/20 4:47 PM, Helge Deller wrote:

>>> -        host_flags = target_to_host_bitmask(arg3, fcntl_flags_tbl);
>>> +        int host_flags = target_to_host_bitmask(arg3, fcntl_flags_tbl);
>>
>> I don't think this is quite correct.  target_to_host_bitmask()
>> silently ignores unknown bits, and a user that was relying on bit
>> 0x40000000 to cause an EINVAL will not fail with this change (unless
>> bit 0x40000000 happens to be one of the bits translated by
>> fcntl_flags_tbl).
> 
> True.
> 
>> The open() syscall is notorious for ignoring unknown bits rather than
>> failing with EINVAL, and it is has come back to haunt kernel
>> developers; newer syscalls like dup3() learned from the mistake, and
>> we really do want to catch unsupported bits up to make it easier for
>> future kernels to define meanings to those bits without them being
>> silently swallowed when run on older systems that did not know what
>> those bits meant.
> Ok, I wasn't aware that it's a design goal to manually find such
> cases of wrong userspace applications. But in this case, you're right
> that my patch shouldn't be applied.

This, and several similar ones that you also posted.

Maybe you could add a new int target_to_host_bitmask_strict(int src, 
translate_tbl, int *dst), which returns 0 when *dst is bit-for-bit 
translated from src, and returns -1 if src had bits not specified by 
translate_tbl.  In that case, the caller can then translate all usual 
bits and rely on the syscall() failure (as you tried here), but you can 
also flag -TARGET_EINVAL up front for bits not covered by the table.

> 
> While looking at the code I just noticed another bug too, which needs
> fixing then:
>>> -        if ((arg3 & ~TARGET_O_CLOEXEC) != 0) {
>>> -            return -EINVAL;
> this needs to be:
>>> -            return -TARGET_EINVAL;

Indeed.  Good catch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org