On Wed, Oct 09, 2019 at 07:11:40PM -0700, Max Filippov wrote:
> > I don't have
> > xtensa cross-toolchain at the moment, so I can't check it easily;
> > what does =r constraint generate in such case?
>
> Lower register of the register pair.
OK...
> > Another thing is, you want to zero it on failure, to avoid an uninitialized
> > value ending up someplace interesting....
>
> Ok, this?
> #define __get_user_nocheck(x, ptr, size) \
> ({ \
> - long __gu_err, __gu_val; \
> + long __gu_err; \
> + __typeof__(*(ptr) + 0) __gu_val = 0; \
> __get_user_size(__gu_val, (ptr), (size), __gu_err); \
> (x) = (__force __typeof__(*(ptr)))__gu_val; \
> __gu_err; \
> @@ -180,7 +181,8 @@ __asm__ __volatile__(
> \
>
> #define __get_user_check(x, ptr, size) \
> ({ \
> - long __gu_err = -EFAULT, __gu_val = 0; \
> + long __gu_err = -EFAULT; \
> + __typeof__(*(ptr) + 0) __gu_val = 0; \
> const __typeof__(*(ptr)) *__gu_addr = (ptr); \
> if (access_ok(__gu_addr, size)) \
> __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> @@ -198,7 +200,7 @@ do {
> \
> case 1: __get_user_asm(x, ptr, retval, 1, "l8ui", __cb); break;\
> case 2: __get_user_asm(x, ptr, retval, 2, "l16ui", __cb); break;\
> case 4: __get_user_asm(x, ptr, retval, 4, "l32i", __cb); break;\
> - case 8: retval = __copy_from_user(&x, ptr, 8); break; \
> + case 8: retval = __copy_from_user(&x, ptr, 8) ? -EFAULT : 0;
> break; \
> default: (x) = __get_user_bad(); \
> } \
> } while (0)
Hmm... Looking at __get_user_size(), we have retval = 0; very early
in it. So I wonder if it should simply be
#define __get_user_size(x, ptr, size, retval) \
do { \
int __cb; \
retval = 0; \
switch (size) { \
case 1: __get_user_asm(x, ptr, retval, 1, "l8ui", __cb); break;\
case 2: __get_user_asm(x, ptr, retval, 2, "l16ui", __cb); break;\
case 4: __get_user_asm(x, ptr, retval, 4, "l32i", __cb); break;\
case 8: if (unlikely(__copy_from_user(&x, ptr, 8)) { \
retval = -EFAULT; \
x = 0; \
} \
break; \
default: (x) = __get_user_bad(); \
} \
} while (0)
so that 64bit case is closer to the others in that respect (i.e. zeroing
done on failure and out of line). No?
