On Thu, Oct 22, 2020 at 9:35 AM David Laight [off-list ref] wrote:

From: Christoph Hellwig

quoted

Sent: 22 October 2020 14:24

On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote:

quoted

My thinking: if the compiler that calls import_iovec() has garbage in
the upper 32 bit

a) gcc will zero it out and not rely on it being zero.
b) clang will not zero it out, assuming it is zero.

But

a) will zero it out when calling the !inlined variant
b) clang will zero it out when calling the !inlined variant

When inlining, b) strikes. We access garbage. That would mean that we
have calling code that's not generated by clang/gcc IIUC.

Most callchains of import_iovec start with the assembly syscall wrappers.

Wait...
readv(2) defines:
        ssize_t readv(int fd, const struct iovec *iov, int iovcnt);

But the syscall is defined as:

SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
                unsigned long, vlen)
{
        return do_readv(fd, vec, vlen, 0);
}

I'm guessing that nothing actually masks the high bits that come
from an application that is compiled with clang?

The vlen is 'unsigned long' through the first few calls.
So unless there is a non-inlined function than takes vlen
as 'int' the high garbage bits from userspace are kept.

Yeah, that's likely a bug: https://godbolt.org/z/KfsPKs

Which makes it a bug in the kernel C syscall wrappers.
They need to explicitly mask the high bits of 32bit
arguments on arm64 but not x86-64.

Why not x86-64? Wouldn't it be *any* LP64 ISA?

Attaching a patch that uses the proper width, but I'm pretty sure
there's still a signedness issue .  Greg, would you mind running this
through the wringer?

What does the ARM EABI say about register parameters?

AAPCS is the ABI for 64b ARM, IIUC, which is the ISA GKH is reporting
the problem against. IIUC, EABI is one of the 32b ABIs.  aarch64 is
LP64 just like x86_64.

--
Thanks,
~Nick Desaulniers