On 22.10.20 14:18, Greg KH wrote: > On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote: >> On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote: >>> On 22.10.20 11:32, David Laight wrote: >>>> From: David Hildenbrand >>>>> Sent: 22 October 2020 10:25 >>>> ... >>>>> ... 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'.... >>>> >>>>> 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
