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

[Eric Blake]

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