From: Al Viro <viro@zeniv.linux.org.uk>
To: Max Filippov <jcmvbkbc@gmail.com>
Cc: "open list:TENSILICA XTENSA PORT (xtensa)"
<linux-xtensa@linux-xtensa.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] xtensa: fix {get,put}_user() for 64bit values
Date: Thu, 10 Oct 2019 15:29:56 +0100 [thread overview]
Message-ID: <20191010142956.GG26530@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAMo8BfLo4hy+WGA7p+7iZaLmmgFOyzMRAtG5dzNj=JEU04GoKA@mail.gmail.com>
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?
next prev parent reply other threads:[~2019-10-10 14:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-09 19:21 [PATCH] xtensa: fix {get,put}_user() for 64bit values Al Viro
2019-10-10 1:38 ` Max Filippov
2019-10-10 1:56 ` Al Viro
2019-10-10 2:11 ` Max Filippov
2019-10-10 14:29 ` Al Viro [this message]
2019-10-11 3:35 ` Max Filippov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191010142956.GG26530@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=jcmvbkbc@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xtensa@linux-xtensa.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.