[PATCH] check put_user fail in do_signal when enable OABI_COMPACT

Mikael Pettersson mikpe at it.uu.se
Tue Oct 27 11:42:43 EDT 2009


Jean Pihet writes:
 > --- a/arch/arm/kernel/signal.c
 > +++ b/arch/arm/kernel/signal.c
 > @@ -643,6 +643,7 @@ static int do_signal(sigset_t *oldset, struct pt_regs 
 > *regs, int syscall)
 >  	struct k_sigaction ka;
 >  	siginfo_t info;
 >  	int signr;
 > +	int ret = 0;
 >  
 >  	/*
 >  	 * We want the common case to go fast, which
 > @@ -685,8 +686,15 @@ static int do_signal(sigset_t *oldset, struct pt_regs 
 > *regs, int syscall)
 >  				regs->ARM_sp -= 4;
 >  				usp = (u32 __user *)regs->ARM_sp;
 >  
 > -				put_user(regs->ARM_pc, usp);
 > -				regs->ARM_pc = KERN_RESTART_CODE;
 > +				ret |= put_user(regs->ARM_pc, usp);
 > +				if (!ret) {
 > +					flush_icache_range((unsigned long)usp,
 > +						(unsigned long)(usp + 1));
 > +					regs->ARM_pc = KERN_RESTART_CODE;
 > +				} else {
 > +					regs->ARM_sp += 4;
 > +					force_sigsegv(0, current);
 > +				}
 >  #endif

I don't like this 'ret' variable:
- it's only for OABI, but declared at the top and initialised also for EABI
- you update it with |=, but clearly there's no useful previous value

I'd drop 'ret' and just do:

				if (put_user(regs->ARM_pc, usp) == 0) {
					flush...
				} else {
					... force_sigsegv
				}

(Note: == 0 not !, since ! is often read as "failed" whereas for
put_user() 0 means Ok and non-zero means -errno.)



More information about the linux-arm-kernel mailing list