Error handling bug in fsys_rt_sigprocmask

Error handling bug in fsys_rt_sigprocmask

[Andreas Schwab]

When sigprocmask is called with an invalid first argument it still
modifies the signal mask of the process.

Andreas.

--- linux-2.6.5/arch/ia64/kernel/fsys.S.~1~	2004-04-04 05:38:28.000000000 +0200
+++ linux-2.6.5/arch/ia64/kernel/fsys.S	2004-05-14 15:43:11.456175248 +0200
@@ -444,6 +444,9 @@ EX(.fail_efault, ld8 r14=[r33])			// r14
 (p7)	cmp.ne.or.andcm p6,p7=r18,r0		// p6/p7 <- signal pending
 	mov r19=0					// i must not leak kernel bits...
 (p6)	br.cond.dpnt.many .sig_pending
+
+	cmp.ne p9,p0=r8,r0			// check for bad HOW value
+(p9)	br.cond.spnt.few 2f
 	;;
 
 1:	ld4 r17=[r9]				// r17 <- current->thread_info->flags
@@ -458,11 +461,11 @@ EX(.fail_efault, ld8 r14=[r33])			// r14
 	cmp.ne p6,p0=r17,r14			// update failed?
 (p6)	br.cond.spnt.few 1b			// yes -> retry
 
+2:
 #ifdef CONFIG_SMP
 	st4.rel [r31]=r0			// release the lock
 #endif
 	ssm psr.i
-	cmp.ne p9,p0=r8,r0			// check for bad HOW value
 	;;
 
 	srlz.d					// ensure psr.i is set again

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Error handling bug in fsys_rt_sigprocmask

[David Mosberger]

>>>>> On Sat, 15 May 2004 22:57:58 +0200, Andreas Schwab <schwab@suse.de> said:

  Andreas> When sigprocmask is called with an invalid first argument
  Andreas> it still modifies the signal mask of the process.

Indeed!  Thanks for catching this.

There already is a check for a bad HOW value, but it comes too late.
Your patch should be safe, but I'd like to fix it in a way that avoids
the second (now unnecessary check).  Let me see what I can come up with.

Thanks,

	--david

Re: Error handling bug in fsys_rt_sigprocmask

[Andreas Schwab]

David Mosberger <davidm@napali.hpl.hp.com> writes:

> There already is a check for a bad HOW value, but it comes too late.
> Your patch should be safe, but I'd like to fix it in a way that avoids
> the second (now unnecessary check).  Let me see what I can come up with.

I did remove the second check.  But there is now an additional bundle and
I couldn't find any way to avoid that.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Error handling bug in fsys_rt_sigprocmask

[David Mosberger]

>>>>> On Thu, 20 May 2004 00:30:34 +0200, Andreas Schwab <schwab@suse.de> said:

  Andreas> I did remove the second check.  But there is now an
  Andreas> additional bundle and I couldn't find any way to avoid
  Andreas> that.

Yeah, we may have to live with that.  It's not really a big deal (in
real life performance, it's not going to make any difference) but it
sure bugs me... ;-)

	--david

Re: Error handling bug in fsys_rt_sigprocmask

[David Mosberger]

>>>>> On Thu, 20 May 2004 00:30:34 +0200, Andreas Schwab <schwab@suse.de> said:

  Andreas> I did remove the second check.  But there is now an
  Andreas> additional bundle and I couldn't find any way to avoid
  Andreas> that.

I think I'm going to go with the attached patch --- unless you find
something wrong with it.

	--david

=== arch/ia64/kernel/fsys.S 1.24 vs edited ==--- 1.24/arch/ia64/kernel/fsys.S	Thu May  6 17:46:45 2004
+++ edited/arch/ia64/kernel/fsys.S	Wed May 19 16:32:39 2004
@@ -362,7 +362,7 @@
 	ld8 r31=[r31]				// r31 <- current->sighand
 #endif
 	and r9=TIF_ALLWORK_MASK,r9
-	tnat.nz p6,p0=r32
+	cmp4.ltu p6,p0=SIG_SETMASK,r32
 	;;
 	cmp.ne p7,p0=0,r9
 	tnat.nz.or p6,p0=r35
@@ -370,6 +370,7 @@
 	;;
 	cmp.ne p15,p0=r0,r34			// oset != NULL?
 	cmp.ne.or p6,p0=_NSIG_WORDS*8,r35
+	tnat.nz.or p6,p0=r32
 	tnat.nz.or p8,p0=r33
 
 (p6)	br.spnt.few .fail_einval		// fail with EINVAL
@@ -462,12 +463,10 @@
 	st4.rel [r31]=r0			// release the lock
 #endif
 	ssm psr.i
-	cmp.ne p9,p0=r8,r0			// check for bad HOW value
 	;;
 
 	srlz.d					// ensure psr.i is set again
 	mov r18=0					// i must not leak kernel bits...
-(p9)	br.spnt.few .fail_einval		// bail out for bad HOW value
 
 .store_mask:
 EX(.fail_efault, (p15) probe.w.fault r34, 3)	// verify user has write-access to *oset