Re: [RFC v2 -tip 3/3] x86: ia32_signal: use {get|put}_user_try and catch

Hiroshi Shimamoto
2009-01-26T18:31:18Z

Cyrill Gorcunov wrote:
> [Hiroshi Shimamoto - Fri, Jan 23, 2009 at 03:50:38PM -0800]
> | From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> |
> | Impact: use new framework
> |
> | Use {get|put}_user_try, catch, and _ex in arch/x86/ia32/ia32_signal.c.
> |
> | Note: this patch contains "WARNING: line over 80 characters", because when
> | introducing new block I insert an indent to avoid mistakes by edit.
> |
> | Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> | ---
> | arch/x86/ia32/ia32_signal.c | 365 +++++++++++++++++++++++--------------------
> | 1 files changed, 195 insertions(+), 170 deletions(-)
> |
> | diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> | index 9dabd00..dd77ac0 100644
> | --- a/arch/x86/ia32/ia32_signal.c
> | +++ b/arch/x86/ia32/ia32_signal.c
> | @@ -46,78 +46,83 @@ void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
> |
> ...
> | + put_user_try {
> | + /* If you change siginfo_t structure, please make sure that
> | + this code is fixed accordingly.
> | + It should never copy any pad contained in the structure
> | + to avoid security leaks, but must copy the generic
> | + 3 ints plus the relevant union member. */
> | + put_user_ex(from->si_signo, &to->si_signo);
> | + put_user_ex(from->si_errno, &to->si_errno);
> | + put_user_ex((short)from->si_code, &to->si_code);
> | +
> | + if (from->si_code < 0) {
> | + put_user_ex(from->si_pid, &to->si_pid);
> | + put_user_ex(from->si_uid, &to->si_uid);
> | + put_user_ex(ptr_to_compat(from->si_ptr), &to->si_ptr);
> | + } else {
> | + /*
> | + * First 32bits of unions are always present:
> | + * si_pid === si_band === si_tid === si_addr(LS half)
> | + */
> | + put_user_ex(from->_sifields._pad[0],
> | + &to->_sifields._pad[0]);
> | + switch (from->si_code >> 16) {
> | + case __SI_FAULT >> 16:
> | + break;
> | + case __SI_CHLD >> 16:
> | + put_user_ex(from->si_utime, &to->si_utime);
> | + put_user_ex(from->si_stime, &to->si_stime);
> | + put_user_ex(from->si_status, &to->si_status);
> | + /* FALL THROUGH */
> | + default:
>
> Hi Hiroshi,

Hi Cyrill,

>
> may I ask why we use default here?

I don't know:) Hm, it looks old code.
arch/i386/kernel/signal.c in 2.4 has similar code.

I guess this code didn't change when copy_siginfo_to_user() was moved
from arch/i386/kernel/signal.c to kernel/signal.c.

Should we change this like copy_siginfo_tu_user() in kernel/signal.c?
Copying si_pid was added in kernel/signal.c.

BTW, it seems same __ST_KILL and default.

Thanks,
Hiroshi

>
> | + case __SI_KILL >> 16:
> | + put_user_ex(from->si_uid, &to->si_uid);
> | + break;
> | + case __SI_POLL >> 16:
> | + put_user_ex(from->si_fd, &to->si_fd);
> | + break;
> | + case __SI_TIMER >> 16:
> | + put_user_ex(from->si_overrun, &to->si_overrun);
> | + put_user_ex(ptr_to_compat(from->si_ptr),
> | + &to->si_ptr);
> | + break;
> | + /* This is not generated by the kernel as of now. */
> | + case __SI_RT >> 16:
> | + case __SI_MESGQ >> 16:
> | + put_user_ex(from->si_uid, &to->si_uid);
> | + put_user_ex(from->si_int, &to->si_int);
> | + break;
> | + }
> | }
> | - }
> | + } put_user_catch(err);
> | +
> | return err;
> | }
> |
> ...
>
> - Cyrill -
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>