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: <20110423175922.GB484@redhat.com>
Date:	Sat, 23 Apr 2011 19:59:22 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Tejun Heo <tj@...nel.org>,
	"Nikita V. Youshchenko" <nyoushchenko@...sta.com>,
	Matt Fleming <matt@...sole-pimps.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH 1/3] signal: sys_rt_sigtimedwait: simplify the timeout logic

No functional changes, cleanup compat_sys_rt_sigtimedwait() and
sys_rt_sigtimedwait().

Calculate the timeout before we take ->siglock, this simplifies and
lessens the code. Use timespec_valid() to check the timespec.

I don't understand why we are adding (ts.tv_sec || ts.tv_nsec) to
timespec_to_jiffies(&ts). Perhaps to ensure we will sleep at least
one jiffy if ts != 0? But in this case we should only check tv_nsec,
I don't think timespec_to_jiffies() can return zero if tv_sec != 0.
In fact I suspect timespec_to_jiffies() can only return zero if
tv_sec == tv_nsec == 0 because we add "TICK_NSEC - 1", but I am not
sure I understand correctly this math.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---

 kernel/signal.c |   46 ++++++++++++++++++++--------------------------
 kernel/compat.c |   38 ++++++++++++++++----------------------
 2 files changed, 36 insertions(+), 48 deletions(-)

--- sigprocmask/kernel/signal.c~1_sigtimedwait_to	2011-04-22 15:48:33.000000000 +0200
+++ sigprocmask/kernel/signal.c	2011-04-22 19:05:51.000000000 +0200
@@ -2327,7 +2327,7 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const s
 	sigset_t these;
 	struct timespec ts;
 	siginfo_t info;
-	long timeout = 0;
+	long timeout;
 
 	/* XXX: Don't preclude handling different sized sigset_t's.  */
 	if (sigsetsize != sizeof(sigset_t))
@@ -2343,41 +2343,35 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const s
 	sigdelsetmask(&these, sigmask(SIGKILL)|sigmask(SIGSTOP));
 	signotset(&these);
 
+	timeout = MAX_SCHEDULE_TIMEOUT;
 	if (uts) {
 		if (copy_from_user(&ts, uts, sizeof(ts)))
 			return -EFAULT;
-		if (ts.tv_nsec >= 1000000000L || ts.tv_nsec < 0
-		    || ts.tv_sec < 0)
+		if (!timespec_valid(&ts))
 			return -EINVAL;
+		timeout = timespec_to_jiffies(&ts) + (ts.tv_sec || ts.tv_nsec);
 	}
 
 	spin_lock_irq(&current->sighand->siglock);
 	sig = dequeue_signal(current, &these, &info);
-	if (!sig) {
-		timeout = MAX_SCHEDULE_TIMEOUT;
-		if (uts)
-			timeout = (timespec_to_jiffies(&ts)
-				   + (ts.tv_sec || ts.tv_nsec));
-
-		if (timeout) {
-			/*
-			 * None ready -- temporarily unblock those we're
-			 * interested while we are sleeping in so that we'll
-			 * be awakened when they arrive.
-			 */
-			current->real_blocked = current->blocked;
-			sigandsets(&current->blocked, &current->blocked, &these);
-			recalc_sigpending();
-			spin_unlock_irq(&current->sighand->siglock);
+	if (!sig && timeout) {
+		/*
+		 * None ready -- temporarily unblock those we're
+		 * interested while we are sleeping in so that we'll
+		 * be awakened when they arrive.
+		 */
+		current->real_blocked = current->blocked;
+		sigandsets(&current->blocked, &current->blocked, &these);
+		recalc_sigpending();
+		spin_unlock_irq(&current->sighand->siglock);
 
-			timeout = schedule_timeout_interruptible(timeout);
+		timeout = schedule_timeout_interruptible(timeout);
 
-			spin_lock_irq(&current->sighand->siglock);
-			sig = dequeue_signal(current, &these, &info);
-			current->blocked = current->real_blocked;
-			siginitset(&current->real_blocked, 0);
-			recalc_sigpending();
-		}
+		spin_lock_irq(&current->sighand->siglock);
+		sig = dequeue_signal(current, &these, &info);
+		current->blocked = current->real_blocked;
+		siginitset(&current->real_blocked, 0);
+		recalc_sigpending();
 	}
 	spin_unlock_irq(&current->sighand->siglock);
 
--- sigprocmask/kernel/compat.c~1_sigtimedwait_to	2011-04-14 21:23:22.000000000 +0200
+++ sigprocmask/kernel/compat.c	2011-04-22 19:19:39.000000000 +0200
@@ -893,7 +893,7 @@ compat_sys_rt_sigtimedwait (compat_sigse
 	int sig;
 	struct timespec t;
 	siginfo_t info;
-	long ret, timeout = 0;
+	long ret, timeout;
 
 	if (sigsetsize != sizeof(sigset_t))
 		return -EINVAL;
@@ -904,36 +904,30 @@ compat_sys_rt_sigtimedwait (compat_sigse
 	sigdelsetmask(&s,sigmask(SIGKILL)|sigmask(SIGSTOP));
 	signotset(&s);
 
+	timeout = MAX_SCHEDULE_TIMEOUT;
 	if (uts) {
 		if (get_compat_timespec (&t, uts))
 			return -EFAULT;
-		if (t.tv_nsec >= 1000000000L || t.tv_nsec < 0
-				|| t.tv_sec < 0)
+		if (!timespec_valid(&t))
 			return -EINVAL;
+		timeout = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec);
 	}
 
 	spin_lock_irq(&current->sighand->siglock);
 	sig = dequeue_signal(current, &s, &info);
-	if (!sig) {
-		timeout = MAX_SCHEDULE_TIMEOUT;
-		if (uts)
-			timeout = timespec_to_jiffies(&t)
-				+(t.tv_sec || t.tv_nsec);
-		if (timeout) {
-			current->real_blocked = current->blocked;
-			sigandsets(&current->blocked, &current->blocked, &s);
-
-			recalc_sigpending();
-			spin_unlock_irq(&current->sighand->siglock);
+	if (!sig && timeout) {
+		current->real_blocked = current->blocked;
+		sigandsets(&current->blocked, &current->blocked, &s);
+		recalc_sigpending();
+		spin_unlock_irq(&current->sighand->siglock);
 
-			timeout = schedule_timeout_interruptible(timeout);
+		timeout = schedule_timeout_interruptible(timeout);
 
-			spin_lock_irq(&current->sighand->siglock);
-			sig = dequeue_signal(current, &s, &info);
-			current->blocked = current->real_blocked;
-			siginitset(&current->real_blocked, 0);
-			recalc_sigpending();
-		}
+		spin_lock_irq(&current->sighand->siglock);
+		sig = dequeue_signal(current, &s, &info);
+		current->blocked = current->real_blocked;
+		siginitset(&current->real_blocked, 0);
+		recalc_sigpending();
 	}
 	spin_unlock_irq(&current->sighand->siglock);
 
@@ -943,7 +937,7 @@ compat_sys_rt_sigtimedwait (compat_sigse
 			if (copy_siginfo_to_user32(uinfo, &info))
 				ret = -EFAULT;
 		}
-	}else {
+	} else {
 		ret = timeout?-EINTR:-EAGAIN;
 	}
 	return ret;

--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ