[2/9] restart_block: Prevent userspace set part of the block

Submitted by Jann Horn via Containers on Sept. 9, 2019, 10:23 a.m.

Details

Message ID 20190909102340.8592-3-dima@arista.com
State New
Series "restart_block: Prepare the ground for dumping timeout"
Headers show

Commit Message

Jann Horn via Containers Sept. 9, 2019, 10:23 a.m.
Parameters for nanosleep() could be chosen the way to make
hrtimer_nanosleep() fail. In that case changes to restarter_block bring
it into inconsistent state. Luckily, it won't corrupt anything critical
for poll() or futex(). But as it's not evident that userspace may do
tricks in the union changing restart_block for other @fs(s) - than
further changes in the code may create a potential local vulnerability.

I.e., if userspace could do tricks with poll() or futex() than
corruption to @clockid or @type would trigger BUG() in timer code.

Set @fn every time restart_block is changed, preventing surprises.
Also, add a comment for any new restart_block user.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/linux/restart_block.h  | 4 ++++
 kernel/time/hrtimer.c          | 8 +++++---
 kernel/time/posix-cpu-timers.c | 6 +++---
 kernel/time/posix-stubs.c      | 8 +++++---
 kernel/time/posix-timers.c     | 8 +++++---
 5 files changed, 22 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/linux/restart_block.h b/include/linux/restart_block.h
index e5078cae5567..e66e982105f4 100644
--- a/include/linux/restart_block.h
+++ b/include/linux/restart_block.h
@@ -21,6 +21,10 @@  enum timespec_type {
 
 /*
  * System call restart block.
+ *
+ * Safety rule: if you change anything inside @restart_block,
+ * set @fn to keep the structure in consistent state and prevent
+ * userspace tricks in the union.
  */
 struct restart_block {
 	long (*fn)(struct restart_block *);
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 5ee77f1a8a92..4ba2b50d068f 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1762,8 +1762,9 @@  SYSCALL_DEFINE2(nanosleep, struct __kernel_timespec __user *, rqtp,
 	if (!timespec64_valid(&tu))
 		return -EINVAL;
 
-	current->restart_block.nanosleep.type = rmtp ? TT_NATIVE : TT_NONE;
-	current->restart_block.nanosleep.rmtp = rmtp;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_NATIVE : TT_NONE;
+	current->restart_block.nanosleep.rmtp	= rmtp;
 	return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
 }
 
@@ -1782,7 +1783,8 @@  SYSCALL_DEFINE2(nanosleep_time32, struct old_timespec32 __user *, rqtp,
 	if (!timespec64_valid(&tu))
 		return -EINVAL;
 
-	current->restart_block.nanosleep.type = rmtp ? TT_COMPAT : TT_NONE;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_COMPAT : TT_NONE;
 	current->restart_block.nanosleep.compat_rmtp = rmtp;
 	return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
 }
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 0a426f4e3125..b4dddf74dd15 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1243,6 +1243,8 @@  void set_process_cpu_timer(struct task_struct *tsk, unsigned int clock_idx,
 	tick_dep_set_signal(tsk->signal, TICK_DEP_BIT_POSIX_TIMER);
 }
 
+static long posix_cpu_nsleep_restart(struct restart_block *restart_block);
+
 static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
 			    const struct timespec64 *rqtp)
 {
@@ -1330,6 +1332,7 @@  static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
 		 * Report back to the user the time still remaining.
 		 */
 		restart = &current->restart_block;
+		restart->fn = posix_cpu_nsleep_restart;
 		restart->nanosleep.expires = expires;
 		if (restart->nanosleep.type != TT_NONE)
 			error = nanosleep_copyout(restart, &it.it_value);
@@ -1338,8 +1341,6 @@  static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
 	return error;
 }
 
-static long posix_cpu_nsleep_restart(struct restart_block *restart_block);
-
 static int posix_cpu_nsleep(const clockid_t which_clock, int flags,
 			    const struct timespec64 *rqtp)
 {
@@ -1361,7 +1362,6 @@  static int posix_cpu_nsleep(const clockid_t which_clock, int flags,
 		if (flags & TIMER_ABSTIME)
 			return -ERESTARTNOHAND;
 
-		restart_block->fn = posix_cpu_nsleep_restart;
 		restart_block->nanosleep.clockid = which_clock;
 	}
 	return error;
diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
index 67df65f887ac..d73039a9ca8f 100644
--- a/kernel/time/posix-stubs.c
+++ b/kernel/time/posix-stubs.c
@@ -142,8 +142,9 @@  SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
-	current->restart_block.nanosleep.type = rmtp ? TT_NATIVE : TT_NONE;
-	current->restart_block.nanosleep.rmtp = rmtp;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_NATIVE : TT_NONE;
+	current->restart_block.nanosleep.rmtp	= rmtp;
 	return hrtimer_nanosleep(&t, flags & TIMER_ABSTIME ?
 				 HRTIMER_MODE_ABS : HRTIMER_MODE_REL,
 				 which_clock);
@@ -228,7 +229,8 @@  SYSCALL_DEFINE4(clock_nanosleep_time32, clockid_t, which_clock, int, flags,
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
-	current->restart_block.nanosleep.type = rmtp ? TT_COMPAT : TT_NONE;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_COMPAT : TT_NONE;
 	current->restart_block.nanosleep.compat_rmtp = rmtp;
 	return hrtimer_nanosleep(&t, flags & TIMER_ABSTIME ?
 				 HRTIMER_MODE_ABS : HRTIMER_MODE_REL,
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index d7f2d91acdac..0ca0bfc20aff 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1189,8 +1189,9 @@  SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
-	current->restart_block.nanosleep.type = rmtp ? TT_NATIVE : TT_NONE;
-	current->restart_block.nanosleep.rmtp = rmtp;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_NATIVE : TT_NONE;
+	current->restart_block.nanosleep.rmtp	= rmtp;
 
 	return kc->nsleep(which_clock, flags, &t);
 }
@@ -1216,7 +1217,8 @@  SYSCALL_DEFINE4(clock_nanosleep_time32, clockid_t, which_clock, int, flags,
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
-	current->restart_block.nanosleep.type = rmtp ? TT_COMPAT : TT_NONE;
+	current->restart_block.fn		= do_no_restart_syscall;
+	current->restart_block.nanosleep.type	= rmtp ? TT_COMPAT : TT_NONE;
 	current->restart_block.nanosleep.compat_rmtp = rmtp;
 
 	return kc->nsleep(which_clock, flags, &t);