linux-kernel - Re: [PATCH 12/44] metag: Use get_signal() signal_setup

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] [day] [month] [year] [list]

Date:	Tue, 18 Mar 2014 11:41:42 +0000
From:	James Hogan <james.hogan@...tec.com>
To:	Richard Weinberger <richard@....at>
CC:	<linux-kernel@...r.kernel.org>, <linux-arch@...r.kernel.org>,
	<viro@...iv.linux.org.uk>, <vgupta@...opsys.com>,
	<catalin.marinas@....com>, <will.deacon@....com>,
	<hskinnemoen@...il.com>, <egtvedt@...fundet.no>,
	<vapier@...too.org>, <msalter@...hat.com>, <a-jacquiot@...com>,
	<starvik@...s.com>, <jesper.nilsson@...s.com>,
	<dhowells@...hat.com>, <rkuo@...eaurora.org>,
	<tony.luck@...el.com>, <fenghua.yu@...el.com>,
	<takata@...ux-m32r.org>, <geert@...ux-m68k.org>,
	<monstr@...str.eu>, <yasutake.koichi@...panasonic.com>,
	<ralf@...ux-mips.org>, <jonas@...thpole.se>,
	<jejb@...isc-linux.org>, <deller@....de>,
	<benh@...nel.crashing.org>, <paulus@...ba.org>,
	<schwidefsky@...ibm.com>, <heiko.carstens@...ibm.com>,
	<liqin.linux@...il.com>, <lennox.wu@...il.com>,
	<cmetcalf@...era.com>, <gxt@...c.pku.edu.cn>,
	<linux-xtensa@...ux-xtensa.org>, <akpm@...ux-foundation.org>,
	<oleg@...hat.com>, <tj@...nel.org>, <hch@...radead.org>,
	linux-metag <linux-metag@...r.kernel.org>
Subject: Re: [PATCH 12/44] metag: Use get_signal() signal_setup_done()

On 03/03/14 11:15, James Hogan wrote:
> Hi Richard,
> 
> On 02/03/14 23:57, Richard Weinberger wrote:
>> @@ -235,10 +230,8 @@ static void handle_signal(unsigned long sig, siginfo_t *info,
>>  static int do_signal(struct pt_regs *regs, int syscall)
>>  {
>>  	unsigned int retval = 0, continue_addr = 0, restart_addr = 0;
>> -	struct k_sigaction ka;
>> -	siginfo_t info;
>> -	int signr;
>>  	int restart = 0;
>> +	struct ksignal ksig;
>>  
>>  	/*
>>  	 * By the end of rt_sigreturn the context describes the point that the
>> @@ -272,30 +265,30 @@ static int do_signal(struct pt_regs *regs, int syscall)
>>  	}
>>  
>>  	/*
>> -	 * Get the signal to deliver. When running under ptrace, at this point
>> -	 * the debugger may change all our registers ...
>> -	 */
>> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
>> -	/*
>>  	 * Depending on the signal settings we may need to revert the decision
>>  	 * to restart the system call. But skip this if a debugger has chosen to
>>  	 * restart at a different PC.
>>  	 */
>>  	if (regs->REG_PC != restart_addr)
>>  		restart = 0;
>> -	if (signr > 0) {
>> +
>> +	/*
>> +	 * Get the signal to deliver. When running under ptrace, at this point
>> +	 * the debugger may change all our registers ...
>> +	 */
>> +	if (get_signal(&ksig)) {
> 
> The patch all looks good to me except for this bit. You've moved the bit
> at which a ptrace debugger can alter the registers to after the check
> for whether the PC has been altered, which would still need to cancel
> the restart.
> 
> Does something like the hunks below look reasonable to you?
> 
> Would you like me to take this patch through the metag tree or do you
> want to keep them together?

I've applied the altered version of this patch to the metag tree for v3.15.

Thanks
James

> 
> Cheers
> James
> 
> @@ -275,7 +268,7 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	 * Get the signal to deliver. When running under ptrace, at this point
>  	 * the debugger may change all our registers ...
>  	 */
> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
> +	get_signal(&ksig);
>  	/*
>  	 * Depending on the signal settings we may need to revert the decision
>  	 * to restart the system call. But skip this if a debugger has chosen to
> @@ -283,19 +276,19 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	 */
>  	if (regs->REG_PC != restart_addr)
>  		restart = 0;
> -	if (signr > 0) {
> +	if (ksig.sig > 0) {
>  		if (unlikely(restart)) {
>  			if (retval == -ERESTARTNOHAND
>  			    || retval == -ERESTART_RESTARTBLOCK
>  			    || (retval == -ERESTARTSYS
> -				&& !(ka.sa.sa_flags & SA_RESTART))) {
> +				&& !(ksig.ka.sa.sa_flags & SA_RESTART))) {
>  				regs->REG_RETVAL = -EINTR;
>  				regs->REG_PC = continue_addr;
>  			}
>  		}
>  
>  		/* Whee! Actually deliver the signal.  */
> -		handle_signal(signr, &info, &ka, regs);
> +		handle_signal(&ksig, regs);
>  		return 0;
>  	}
>  
> 


Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)