On 22.10.20 14:18, Greg KH wrote:

On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote:

quoted

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

quoted

On 22.10.20 11:32, David Laight wrote:

quoted

From: David Hildenbrand

quoted

Sent: 22 October 2020 10:25

...

quoted

... especially because I recall that clang and gcc behave slightly
differently:

https://github.com/hjl-tools/x86-psABI/issues/2

"Function args are different: narrow types are sign or zero extended to
32 bits, depending on their type. clang depends on this for incoming
args, but gcc doesn't make that assumption. But both compilers do it
when calling, so gcc code can call clang code.

It really is best to use 'int' (or even 'long') for all numeric
arguments (and results) regardless of the domain of the value.

Related, I've always worried about 'bool'....

quoted

The upper 32 bits of registers are always undefined garbage for types
smaller than 64 bits."

On x86-64 the high bits are zeroed by all 32bit loads.

Yeah, but does not help here.


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.

We can test easily by changing the parameters instead of adding an "inline".

Let me try that as well, as I seem to have a good reproducer, but it
takes a while to run...

Ok, that didn't work.

And I can't seem to "fix" this by adding noinline to patches further
along in the patch series (because this commit's function is no longer
present due to later patches.)

We might have the same issues with iovec_from_user() and friends now.

Will keep digging...

greg k-h

Might be worth to give this a try, just to see if it's related to
garbage in upper 32 bit and the way clang is handling it (might be a BUG
in clang, though):

diff --git a/include/linux/uio.h b/include/linux/uio.h
index 72d88566694e..7527298c6b56 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h

@@ -267,7 +267,7 @@ size_t hash_and_copy_to_iter(const void *addr,

size_t bytes, void *hashp,
                struct iov_iter *i);

 struct iovec *iovec_from_user(const struct iovec __user *uvector,
-               unsigned long nr_segs, unsigned long fast_segs,
+               unsigned nr_segs, unsigned fast_segs,
                struct iovec *fast_iov, bool compat);
 ssize_t import_iovec(int type, const struct iovec __user *uvec,
                 unsigned nr_segs, unsigned fast_segs, struct iovec **iovp,

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 1635111c5bd2..58417f1916dc 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c

@@ -1652,7 +1652,7 @@ const void *dup_iter(struct iov_iter *new, struct

iov_iter *old, gfp_t flags)
 EXPORT_SYMBOL(dup_iter);

 static int copy_compat_iovec_from_user(struct iovec *iov,
-               const struct iovec __user *uvec, unsigned long nr_segs)
+               const struct iovec __user *uvec, unsigned nr_segs)
 {
        const struct compat_iovec __user *uiov =
                (const struct compat_iovec __user *)uvec;

@@ -1684,7 +1684,7 @@ static int copy_compat_iovec_from_user(struct

iovec *iov,
 }

 static int copy_iovec_from_user(struct iovec *iov,
-               const struct iovec __user *uvec, unsigned long nr_segs)
+               const struct iovec __user *uvec, unsigned nr_segs)
 {
        unsigned long seg;

@@ -1699,7 +1699,7 @@ static int copy_iovec_from_user(struct iovec *iov,
 }

 struct iovec *iovec_from_user(const struct iovec __user *uvec,
-               unsigned long nr_segs, unsigned long fast_segs,
+               unsigned nr_segs, unsigned fast_segs,
                struct iovec *fast_iov, bool compat)
 {
        struct iovec *iov = fast_iov;

@@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct

iovec __user *uvec,
                 struct iov_iter *i, bool compat)
 {
        ssize_t total_len = 0;
-       unsigned long seg;
+       unsigned seg;
        struct iovec *iov;

        iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);


-- 
Thanks,

David / dhildenb