linux-kernel - Re: [RFC v2 -tip 3/3] x86: ia32_signal: use {get|put}_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090124073638.GA8752@localhost>
Date:	Sat, 24 Jan 2009 10:36:38 +0300
From:	Cyrill Gorcunov <gorcunov@...il.com>
To:	Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC v2 -tip 3/3] x86: ia32_signal: use {get|put}_user_try and
	catch

[Hiroshi Shimamoto - Fri, Jan 23, 2009 at 03:50:38PM -0800]
| From: Hiroshi Shimamoto <h-shimamoto@...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@...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,

may I ask why we use default here?

| +			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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/