linux-next: build failure after merge of the aio tree

linux-next: build failure after merge of the aio tree

[Stephen Rothwell]

Hi Benjamin,

On Tue, 12 Jan 2016 11:38:35 -0500 Benjamin LaHaise <bcrl@kvack.org> wrote:
>
> On Tue, Jan 12, 2016 at 04:40:34PM +1100, Stephen Rothwell wrote:
> > 
> > After merging the aio tree, today's linux-next build (arm
> > multi_v7_defconfig) failed like this:
> > 
> > fs/built-in.o: In function `aio_thread_op_foo_at':
> > file.c:(.text+0x43808): undefined reference to `__get_user_bad'
> > file.c:(.text+0x43838): undefined reference to `__get_user_bad'  
> 
> This is very strange.  It seems to imply that __get_user() doesn't 
> handle 64 bit values, which is completely broken behaviour on the 
> architecture's part if true.  Can any ARM folks comment on the right 
> fix here?

Well, probably only if you cc them :-)

Indeed, __get_user on arm does not handle 64 bit objects, where as
get_user does ...

Background: new aio code is adding __get_user() calls referencing 64
bit quantities (__u64 and __s64).

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au

linux-next: build failure after merge of the aio tree

[Russell King - ARM Linux]

On Wed, Jan 27, 2016 at 01:40:24PM +1100, Stephen Rothwell wrote:
> Hi Benjamin,
> 
> On Tue, 12 Jan 2016 11:38:35 -0500 Benjamin LaHaise <bcrl@kvack.org> wrote:
> >
> > On Tue, Jan 12, 2016 at 04:40:34PM +1100, Stephen Rothwell wrote:
> > > 
> > > After merging the aio tree, today's linux-next build (arm
> > > multi_v7_defconfig) failed like this:
> > > 
> > > fs/built-in.o: In function `aio_thread_op_foo_at':
> > > file.c:(.text+0x43808): undefined reference to `__get_user_bad'
> > > file.c:(.text+0x43838): undefined reference to `__get_user_bad'  
> > 
> > This is very strange.  It seems to imply that __get_user() doesn't 
> > handle 64 bit values, which is completely broken behaviour on the 
> > architecture's part if true.  Can any ARM folks comment on the right 
> > fix here?
> 
> Well, probably only if you cc them :-)
> 
> Indeed, __get_user on arm does not handle 64 bit objects, where as
> get_user does ...

I believe adding 64-bit support to __get_user() is going to be a really
painful exercise.  It took long enough to make get_user() accept it
without creating any new warnings, and then bug fix it for big endian.
__get_user() has the added complexity that it's inline assembler, and
we can't play the same games that we used in __get_user().

It's not a simple case of:

 #define __get_user_err(x, ptr, err)					\
 do {									\
 	unsigned long __gu_addr = (unsigned long)(ptr); 		\
-	unsigned long __gu_val; 					\
+	unsigned long long __gu_val; 					\
 	unsigned int __ua_flags;					\
 	__chk_user_ptr(ptr);						\
 	might_fault();							\
 	__ua_flags = uaccess_save_and_enable(); 			\
 	switch (sizeof(*(ptr))) {					\
 	case 1: __get_user_asm_byte(__gu_val, __gu_addr, err);	break;	\
 	case 2: __get_user_asm_half(__gu_val, __gu_addr, err);	break;	\
 	case 4: __get_user_asm_word(__gu_val, __gu_addr, err);	break;	\
+	case 8: __get_user_asm_dword(__gu_val, __gu_addr, err); break;	\
 	default: (__gu_val) = __get_user_bad(); 			\
 	}								\
 	uaccess_restore(__ua_flags);					\
 	(x) = (__typeof__(*(ptr)))__gu_val;				\
 } while (0)

as, while that works for integer 'x', it doesn't work when 'x' is a
pointer type, as the cast on the last line creates a GCC warning
about casting to a different size.

You can't move the cast into the switch to eliminate that, because
GCC still warns even though the code is unreachable.  Same problem
exists if you move the variable declaration inside the switch.

> Background: new aio code is adding __get_user() calls referencing 64
> bit quantities (__u64 and __s64).

There's lots more architectures which do not support 64-bit get_user()
_or_ __get_user(): avr32, blackfin, metag for example, and m68k which
has this interesting thing "/* case 8: disabled because gcc-4.1 has a
broken typeof \" in its *get_user() implementation.

Even x86-32 doesn't support it:

#define __get_user(x, ptr)                                              \
        __get_user_nocheck((x), (ptr), sizeof(*(ptr)))

#define __get_user_nocheck(x, ptr, size)                                \
({                                                                      \
        int __gu_err;                                                   \
        unsigned long __gu_val;                                         \
        __uaccess_begin();                                              \
        __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);    \
        __uaccess_end();                                                \
        (x) = (__force __typeof__(*(ptr)))__gu_val;                     \
        __builtin_expect(__gu_err, 0);                                  \
})

#define __get_user_size(x, ptr, size, retval, errret)                   \
do {                                                                    \
        retval = 0;                                                     \
        __chk_user_ptr(ptr);                                            \
        switch (size) {                                                 \
...
        case 8:                                                         \
                __get_user_asm_u64(x, ptr, retval, errret);             \
                break;                                                  \
        default:                                                        \
                (x) = __get_user_bad();                                 \
        }                                                               \
} while (0)

#ifdef CONFIG_X86_32
#define __get_user_asm_u64(x, ptr, retval, errret)      (x) = __get_user_bad()
#define __get_user_asm_ex_u64(x, ptr)                   (x) = __get_user_bad()

Note that __get_user_nocheck() above always uses a 32-bit integer for
__gu_val, so fixing x86-32 will not be a case of just providing a
better __get_user_asm_u64() - it's the same problem that I have on ARM.

The problem for 32-bit architectures is being able to cope sanely with:
* different endians on the same arch
* gcc warning about casting to different sizes, even when the code is
  unreachable
* preserving proper and correct typechecking in get_user() and friends

So, I suspect if AIO were to run a test build on x86-32, they would find
the same errors.

Anyone working on this needs to ensure that any implementation builds
without warning on all of these:

int test_8(unsigned char *v, unsigned char *p)
{
        return __get_user(*v, p);
}

int test_8_constp(unsigned char *v, const unsigned char *p)
{
        return __get_user(*v, p);
}

int test_8_volatilep(unsigned char *v, volatile unsigned char *p)
{
        return __get_user(*v, p);
}

int test_16(unsigned short *v, unsigned short *p)
{
        return __get_user(*v, p);
}

int test_16_constp(unsigned short *v, const unsigned short *p)
{
        return __get_user(*v, p);
}

int test_32(unsigned int *v, unsigned int *p)
{
        return __get_user(*v, p);
}

int test_32_constp(unsigned int *v, const unsigned int *p)
{
        return __get_user(*v, p);
}

int test_64(unsigned long long *v, unsigned long long *p)
{
        return __get_user(*v, p);
}

int test_64_constp(unsigned long long *v, const unsigned long long *p)
{
        return __get_user(*v, p);
}

int test_ptr(unsigned int **v, unsigned int **p)
{
        return __get_user(*v, p);
}

int test_const(unsigned int *v, const unsigned int *p)
{
        return __get_user(*v, p);
}

int test_64_narrow(unsigned long *v, unsigned long long *p)
{
        return __get_user(*v, p);
}

int test_32_wide(unsigned long long *v, unsigned long *p)
{
        return __get_user(*v, p);
}

except for this which must warn:

int test_wrong(char **v, const char **p)
{
        return __get_user(*v, p);
}

which comes from my test set for when get_user() was being worked on.

So, in summary, it seems that the 32-bit architectures I've looked at
so far do not support 64-bit __get_user(), even the popular ones.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

linux-next: build failure after merge of the aio tree

[Geert Uytterhoeven]

On Fri, Jan 29, 2016 at 12:30 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>> Background: new aio code is adding __get_user() calls referencing 64
>> bit quantities (__u64 and __s64).
>
> There's lots more architectures which do not support 64-bit get_user()
> _or_ __get_user(): avr32, blackfin, metag for example, and m68k which
> has this interesting thing "/* case 8: disabled because gcc-4.1 has a
> broken typeof \" in its *get_user() implementation.

And if you enable it again, you get lots of "warning: cast to pointer from
integer of different size", like you mentioned.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

linux-next: build failure after merge of the aio tree

[Stephen Rothwell]

Hi Benjamin,

On Fri, 29 Jan 2016 13:03:39 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Fri, Jan 29, 2016 at 12:30 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> >> Background: new aio code is adding __get_user() calls referencing 64
> >> bit quantities (__u64 and __s64).  
> >
> > There's lots more architectures which do not support 64-bit get_user()
> > _or_ __get_user(): avr32, blackfin, metag for example, and m68k which
> > has this interesting thing "/* case 8: disabled because gcc-4.1 has a
> > broken typeof \" in its *get_user() implementation.  
> 
> And if you enable it again, you get lots of "warning: cast to pointer from
> integer of different size", like you mentioned.

Any thoughts?  I am still using the version of tha aio tree from
next-20160111.

-- 
Cheers,
Stephen Rothwell

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 01:19:59PM +1100, Stephen Rothwell wrote:
> Hi Benjamin,
> 
> On Fri, 29 Jan 2016 13:03:39 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > On Fri, Jan 29, 2016 at 12:30 PM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> > >> Background: new aio code is adding __get_user() calls referencing 64
> > >> bit quantities (__u64 and __s64).  
> > >
> > > There's lots more architectures which do not support 64-bit get_user()
> > > _or_ __get_user(): avr32, blackfin, metag for example, and m68k which
> > > has this interesting thing "/* case 8: disabled because gcc-4.1 has a
> > > broken typeof \" in its *get_user() implementation.  
> > 
> > And if you enable it again, you get lots of "warning: cast to pointer from
> > integer of different size", like you mentioned.
> 
> Any thoughts?  I am still using the version of tha aio tree from
> next-20160111.

I am still convinced that this is an architecture issue.  Given that 64 bit 
values work in the *get_user implementations on other architectures, I see 
no reason there should need to be a workaround for this in common code.

		-ben

> -- 
> Cheers,
> Stephen Rothwell

-- 
"Thought is the essence of where you are now."

linux-next: build failure after merge of the aio tree

[Russell King - ARM Linux]

On Thu, Feb 04, 2016 at 08:41:42AM -0500, Benjamin LaHaise wrote:
> On Thu, Feb 04, 2016 at 01:19:59PM +1100, Stephen Rothwell wrote:
> > Hi Benjamin,
> > 
> > On Fri, 29 Jan 2016 13:03:39 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >
> > > On Fri, Jan 29, 2016 at 12:30 PM, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > > >> Background: new aio code is adding __get_user() calls referencing 64
> > > >> bit quantities (__u64 and __s64).  
> > > >
> > > > There's lots more architectures which do not support 64-bit get_user()
> > > > _or_ __get_user(): avr32, blackfin, metag for example, and m68k which
> > > > has this interesting thing "/* case 8: disabled because gcc-4.1 has a
> > > > broken typeof \" in its *get_user() implementation.  
> > > 
> > > And if you enable it again, you get lots of "warning: cast to pointer from
> > > integer of different size", like you mentioned.
> > 
> > Any thoughts?  I am still using the version of tha aio tree from
> > next-20160111.
> 
> I am still convinced that this is an architecture issue.  Given that 64 bit 
> values work in the *get_user implementations on other architectures, I see 
> no reason there should need to be a workaround for this in common code.

So you're happy to break x86-32 then...

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 01:50:56PM +0000, Russell King - ARM Linux wrote:
> > I am still convinced that this is an architecture issue.  Given that 64 bit 
> > values work in the *get_user implementations on other architectures, I see 
> > no reason there should need to be a workaround for this in common code.
> 
> So you're happy to break x86-32 then...

x86-32 works fine.

		-ben
-- 
"Thought is the essence of where you are now."

linux-next: build failure after merge of the aio tree

[Russell King - ARM Linux]

On Thu, Feb 04, 2016 at 09:08:22AM -0500, Benjamin LaHaise wrote:
> On Thu, Feb 04, 2016 at 01:50:56PM +0000, Russell King - ARM Linux wrote:
> > > I am still convinced that this is an architecture issue.  Given that 64 bit 
> > > values work in the *get_user implementations on other architectures, I see 
> > > no reason there should need to be a workaround for this in common code.
> > 
> > So you're happy to break x86-32 then...
> 
> x86-32 works fine.

Let me repeat the quote from my previous email:

#define __get_user(x, ptr)                                              \
        __get_user_nocheck((x), (ptr), sizeof(*(ptr)))

#define __get_user_nocheck(x, ptr, size)                                \
({                                                                      \
        int __gu_err;                                                   \
        unsigned long __gu_val;                                         \
        __uaccess_begin();                                              \
        __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);    \
        __uaccess_end();                                                \
        (x) = (__force __typeof__(*(ptr)))__gu_val;                     \
        __builtin_expect(__gu_err, 0);                                  \
})

#define __get_user_size(x, ptr, size, retval, errret)                   \
do {                                                                    \
        retval = 0;                                                     \
        __chk_user_ptr(ptr);                                            \
        switch (size) {                                                 \
        case 1:                                                         \
                __get_user_asm(x, ptr, retval, "b", "b", "=q", errret); \
                break;                                                  \
        case 2:                                                         \
                __get_user_asm(x, ptr, retval, "w", "w", "=r", errret); \
                break;                                                  \
        case 4:                                                         \
                __get_user_asm(x, ptr, retval, "l", "k", "=r", errret); \
                break;                                                  \
        case 8:                                                         \
                __get_user_asm_u64(x, ptr, retval, errret);             \
                break;                                                  \
        default:                                                        \
                (x) = __get_user_bad();                                 \
        }                                                               \
} while (0)

#ifdef CONFIG_X86_32
#define __get_user_asm_u64(x, ptr, retval, errret)      (x) = __get_user_bad()
#define __get_user_asm_ex_u64(x, ptr)                   (x) = __get_user_bad()
#else
#define __get_user_asm_u64(x, ptr, retval, errret) \
         __get_user_asm(x, ptr, retval, "q", "", "=r", errret)
#define __get_user_asm_ex_u64(x, ptr) \
         __get_user_asm_ex(x, ptr, "q", "", "=r")
#endif

Hence, __get_user() on x86-32 with a 64-bit quantity results in
__get_user_bad() being called, which is an undefined function.
Only if you build with x86-64 support enabled (iow, CONFIG_X86_32 not
defined) then you get the 64-bit __get_user() support.

Given this, I fail to see how x86-32 can possibly work.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 02:12:53PM +0000, Russell King - ARM Linux wrote:
> Hence, __get_user() on x86-32 with a 64-bit quantity results in
> __get_user_bad() being called, which is an undefined function.
> Only if you build with x86-64 support enabled (iow, CONFIG_X86_32 not
> defined) then you get the 64-bit __get_user() support.
> 
> Given this, I fail to see how x86-32 can possibly work.

You're right; mea culpa.  It compiles without warning on x86-32, but it 
does not link.  I still think this is broken archtecture stupidity since 
put_user() works for 64 bit data types.

		-ben
-- 
"Thought is the essence of where you are now."

linux-next: build failure after merge of the aio tree

[Russell King - ARM Linux]

On Thu, Feb 04, 2016 at 09:32:04AM -0500, Benjamin LaHaise wrote:
> On Thu, Feb 04, 2016 at 02:12:53PM +0000, Russell King - ARM Linux wrote:
> > Hence, __get_user() on x86-32 with a 64-bit quantity results in
> > __get_user_bad() being called, which is an undefined function.
> > Only if you build with x86-64 support enabled (iow, CONFIG_X86_32 not
> > defined) then you get the 64-bit __get_user() support.
> > 
> > Given this, I fail to see how x86-32 can possibly work.
> 
> You're right; mea culpa.  It compiles without warning on x86-32, but it 
> does not link.  I still think this is broken archtecture stupidity since 
> put_user() works for 64 bit data types.

Indeed, and you'll find that several other architectures besides ARM and
x86-32 have exactly the same problem - as I listed in my message from a
few days ago.

Okay, so now I get to set you a challenge, since you're the one wanting
64-bit __get_user(): try implementing it on x86-32 :)

Also in my previous message from a few days ago I provided a set of
functions which test out the implementation.  Here they are... again.

All these should not produce any warnings, and should produce correct
code - especially the narrowing/widening tests:

int test_8(unsigned char *v, unsigned char *p)
{ return __get_user(*v, p); }
int test_8_constp(unsigned char *v, const unsigned char *p)
{ return __get_user(*v, p); }
int test_8_volatilep(unsigned char *v, volatile unsigned char *p)
{ return __get_user(*v, p); }
int test_16(unsigned short *v, unsigned short *p)
{ return __get_user(*v, p); }
int test_16_constp(unsigned short *v, const unsigned short *p)
{ return __get_user(*v, p); }
int test_32(unsigned int *v, unsigned int *p)
{ return __get_user(*v, p); }
int test_32_constp(unsigned int *v, const unsigned int *p)
{ return __get_user(*v, p); }
int test_64(unsigned long long *v, unsigned long long *p)
{ return __get_user(*v, p); }
int test_64_constp(unsigned long long *v, const unsigned long long *p)
{ return __get_user(*v, p); }
int test_ptr(unsigned int **v, unsigned int **p)
{ return __get_user(*v, p); }
int test_const(unsigned int *v, const unsigned int *p)
{ return __get_user(*v, p); }
int test_64_narrow(unsigned long *v, unsigned long long *p)
{ return __get_user(*v, p); }
int test_32_wide(unsigned long long *v, unsigned long *p)
{ return __get_user(*v, p); }

However, this one should warn:

int test_wrong(char **v, const char **p)
{ return __get_user(*v, p); }

Good luck (I think you'll need lots of it to get a working solution)! :)

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 02:39:07PM +0000, Russell King - ARM Linux wrote:
> However, this one should warn:
> 
> int test_wrong(char **v, const char **p)
> { return __get_user(*v, p); }
> 
> Good luck (I think you'll need lots of it to get a working solution)! :)

This works with your test cases on x86-32.  Note that it's only compile + 
link tested at present.

		-ben
-- 
"Thought is the essence of where you are now."

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 09b1b0a..d8834c2 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -326,7 +326,21 @@ do {									\
 } while (0)
 
 #ifdef CONFIG_X86_32
-#define __get_user_asm_u64(x, ptr, retval, errret)	(x) = __get_user_bad()
+#define __get_user_asm_u64(x, addr, err, errret)			\
+	asm volatile(ASM_STAC "\n"					\
+		     "1:	movl %2,%%eax\n"			\
+		     "		movl %3,%%edx\n"			\
+		     "2: " ASM_CLAC "\n"				\
+		     ".section .fixup,\"ax\"\n"				\
+		     "3:	mov %4,%0\n"				\
+		     "	xorl %%eax,%%eax\n"				\
+		     "	xorl %%edx,%%edx\n"				\
+		     "	jmp 2b\n"					\
+		     ".previous\n"					\
+		     _ASM_EXTABLE(1b, 3b)				\
+		     : "=r" (err), "=A"(x)				\
+		     : "m" (__m(addr)), "m" __m(((u32 *)addr) + 1), "i" (errret), "0" (err))
+
 #define __get_user_asm_ex_u64(x, ptr)			(x) = __get_user_bad()
 #else
 #define __get_user_asm_u64(x, ptr, retval, errret) \

linux-next: build failure after merge of the aio tree

[Russell King - ARM Linux]

On Thu, Feb 04, 2016 at 11:01:01AM -0500, Benjamin LaHaise wrote:
> On Thu, Feb 04, 2016 at 02:39:07PM +0000, Russell King - ARM Linux wrote:
> > However, this one should warn:
> > 
> > int test_wrong(char **v, const char **p)
> > { return __get_user(*v, p); }
> > 
> > Good luck (I think you'll need lots of it to get a working solution)! :)
> 
> This works with your test cases on x86-32.  Note that it's only compile + 
> link tested at present.

That's the easy bit!

The problem you're going to run into is here:

#define __get_user_nocheck(x, ptr, size)                                \
({                                                                      \
        int __gu_err;                                                   \
        unsigned long __gu_val;                                         \
        __uaccess_begin();                                              \
        __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);    \
        __uaccess_end();                                                \
        (x) = (__force __typeof__(*(ptr)))__gu_val;                     \

__gu_val will be 32-bit, even when you're wanting a 64-bit quantity.
That's where the fun and games start...

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 04:17:42PM +0000, Russell King - ARM Linux wrote:
> On Thu, Feb 04, 2016 at 11:01:01AM -0500, Benjamin LaHaise wrote:
> > On Thu, Feb 04, 2016 at 02:39:07PM +0000, Russell King - ARM Linux wrote:
> > > However, this one should warn:
> > > 
> > > int test_wrong(char **v, const char **p)
> > > { return __get_user(*v, p); }
> > > 
> > > Good luck (I think you'll need lots of it to get a working solution)! :)
> > 
> > This works with your test cases on x86-32.  Note that it's only compile + 
> > link tested at present.
> 
> That's the easy bit!
> 
> The problem you're going to run into is here:
> 
> #define __get_user_nocheck(x, ptr, size)                                \
> ({                                                                      \
>         int __gu_err;                                                   \
>         unsigned long __gu_val;                                         \
>         __uaccess_begin();                                              \
>         __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);    \
>         __uaccess_end();                                                \
>         (x) = (__force __typeof__(*(ptr)))__gu_val;                     \
> 
> __gu_val will be 32-bit, even when you're wanting a 64-bit quantity.
> That's where the fun and games start...

Ugh.  You're making me install a 32 bit distro.....!

		-ben

> -- 
> RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

-- 
"Thought is the essence of where you are now."

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 04:17:42PM +0000, Russell King - ARM Linux wrote:
> That's the easy bit!
> 
> The problem you're going to run into is here:
> 
> #define __get_user_nocheck(x, ptr, size)                                \
> ({                                                                      \
>         int __gu_err;                                                   \
>         unsigned long __gu_val;                                         \
>         __uaccess_begin();                                              \
>         __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);    \
>         __uaccess_end();                                                \
>         (x) = (__force __typeof__(*(ptr)))__gu_val;                     \
> 
> __gu_val will be 32-bit, even when you're wanting a 64-bit quantity.
> That's where the fun and games start...

You're right -- it's quite non-trivial.  How evil would it be to make a 
separate __get_user64() macro?

		-ben
-- 
"Thought is the essence of where you are now."

linux-next: build failure after merge of the aio tree

[Benjamin LaHaise]

On Thu, Feb 04, 2016 at 04:17:42PM +0000, Russell King - ARM Linux wrote:
> __gu_val will be 32-bit, even when you're wanting a 64-bit quantity.
> That's where the fun and games start...

Okay, I figured out how to do it: instead of using a 64 bit unsigned long 
long for __gu_val, use an array of 2 unsigned longs.  See the patch below 
which I verified boots, passes your tests and doesn't truncate 64 bit 
values.  Comments?  A simple test module to verify things is located at 
http://www.kvack.org/~bcrl/test_module2.c .

		-ben
-- 
"Thought is the essence of where you are now."


diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 09b1b0a..53244ae 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -326,7 +326,22 @@ do {									\
 } while (0)
 
 #ifdef CONFIG_X86_32
-#define __get_user_asm_u64(x, ptr, retval, errret)	(x) = __get_user_bad()
+#define __get_user_asm_u64(x, addr, err, errret)			\
+	asm volatile(ASM_STAC "\n"					\
+		     "1:	movl %2,%%eax\n"			\
+		     "2:	movl %3,%%edx\n"			\
+		     "3: " ASM_CLAC "\n"				\
+		     ".section .fixup,\"ax\"\n"				\
+		     "4:	mov %4,%0\n"				\
+		     "	xorl %%eax,%%eax\n"				\
+		     "	xorl %%edx,%%edx\n"				\
+		     "	jmp 3b\n"					\
+		     ".previous\n"					\
+		     _ASM_EXTABLE(1b, 4b)				\
+		     _ASM_EXTABLE(2b, 4b)				\
+		     : "=r" (err), "=A"(x)				\
+		     : "m" (__m(addr)), "m" __m(((u32 *)addr) + 1), "i" (errret), "0" (err))
+
 #define __get_user_asm_ex_u64(x, ptr)			(x) = __get_user_bad()
 #else
 #define __get_user_asm_u64(x, ptr, retval, errret) \
@@ -407,9 +422,16 @@ do {									\
 #define __get_user_nocheck(x, ptr, size)				\
 ({									\
 	int __gu_err;							\
-	unsigned long __gu_val;						\
-	__get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT);	\
-	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
+	if (size == 8) {						\
+		unsigned long __gu_val[2];				\
+		__gu_err = 0;						\
+		__get_user_asm_u64(__gu_val, ptr, __gu_err, -EFAULT);	\
+		(x) = *(__force __typeof__((ptr)))__gu_val;		\
+	} else {							\
+		unsigned long __gu_val;					\
+		__get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT); \
+		(x) = (__force __typeof__(*(ptr)))__gu_val;		\
+	}								\
 	__builtin_expect(__gu_err, 0);					\
 })