Messages in this thread | | | Date | Thu, 15 Oct 2015 20:41:58 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 04/20] x86: Rewrite copy_siginfo_{to,from}_user32 |
| |
OOH ;) I'll try to look at this patch and the changes in the generic code later. A couple of nits right now.
Please CC x86 maintainers, not only x86@kernel.org.
Please do not remove get/put_user_ex from this code. And this reminds me that we can improve *user_try/*user_catch ...
On 10/14, Amanieu d'Antras wrote: > > -int copy_siginfo_to_user32(compat_siginfo_t __user *to, const siginfo_t *from) > +int copy_siginfo_to_user32(struct compat_siginfo __user *to, const siginfo_t *from) > { > - int err = 0; > + int err, si_code; > bool ia32 = test_thread_flag(TIF_IA32); > > - if (!access_ok(VERIFY_WRITE, to, sizeof(compat_siginfo_t))) > + if (!access_ok(VERIFY_WRITE, to, sizeof(siginfo_t)))
Why? This looks wrong.
> + if (from->si_code < 0) { > + err |= __copy_to_user(to->_sifields._pad, from->_sifields._pad, SI_PAD_SIZE * sizeof(int)) > + ? -EFAULT : 0; > + return err;
I think you should split this patch. And this change (don't interpet, just copy) should go as a separate change.
> + switch (from->si_code & __SI_MASK) { > + case __SI_KILL:
I agree, this looks better than ">> 16", but I'd suggest a separate change too.
[...snip...]
the rest looks unreviewable because you didn't split it and because you removed try/catch ;) The same for copy-from-user.
Please help us to understand these changes and make the more reviewable patches if possible. Personally I think you have a point.
Oleg.
| |