[PATCH] futex: Change 'utime' parameter to be 'const ... *'

* [PATCH] futex: Change 'utime' parameter to be 'const ... *'
@ 2020-11-28 12:39 Alejandro Colomar
  2020-12-10 17:36 ` Alejandro Colomar (man-pages)
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alejandro Colomar @ 2020-11-28 12:39 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: Alejandro Colomar, Peter Zijlstra, Darren Hart, linux-kernel, linux-man

futex(2) says that 'utime' is a pointer to 'const'.
The implementation doesn't use 'const';
however, it _never_ modifies the contents of utime.

- futex() either uses 'utime' as a pointer to struct or as a 'u32'.

- In case it's used as a 'u32', it makes a copy of it,
  and of course it is not dereferenced.

- In case it's used as a 'struct __kernel_timespec __user *',
  the pointer is not dereferenced inside the futex() definition,
  and it is only passed to a function: get_timespec64(),
  which accepts a 'const struct __kernel_timespec __user *'.

context:
........

[[
FUTEX(2)               Linux Programmer's Manual              FUTEX(2)

NAME
       futex - fast user-space locking

SYNOPSIS
       #include <linux/futex.h>
       #include <stdint.h>
       #include <sys/time.h>

       long futex(uint32_t *uaddr, int futex_op, uint32_t val,
                 const struct timespec *timeout,   /* or: uint32_t val2 */
                 uint32_t *uaddr2, uint32_t val3);

       Note:  There  is  no  glibc  wrapper  for this system call; see
       NOTES.
]]

$ sed -n '/SYSCALL_DEFINE.(futex\>/,/^}/p' linux/kernel/futex.c
SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
		struct __kernel_timespec __user *, utime, u32 __user *, uaddr2,
		u32, val3)
{
	struct timespec64 ts;
	ktime_t t, *tp = NULL;
	u32 val2 = 0;
	int cmd = op & FUTEX_CMD_MASK;

	if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
		      cmd == FUTEX_WAIT_BITSET ||
		      cmd == FUTEX_WAIT_REQUEUE_PI)) {
		if (unlikely(should_fail_futex(!(op & FUTEX_PRIVATE_FLAG))))
			return -EFAULT;
		if (get_timespec64(&ts, utime))
			return -EFAULT;
		if (!timespec64_valid(&ts))
			return -EINVAL;

		t = timespec64_to_ktime(ts);
		if (cmd == FUTEX_WAIT)
			t = ktime_add_safe(ktime_get(), t);
		else if (!(op & FUTEX_CLOCK_REALTIME))
			t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
		tp = &t;
	}
	/*
	 * requeue parameter in 'utime' if cmd == FUTEX_*_REQUEUE_*.
	 * number of waiters to wake in 'utime' if cmd == FUTEX_WAKE_OP.
	 */
	if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE ||
	    cmd == FUTEX_CMP_REQUEUE_PI || cmd == FUTEX_WAKE_OP)
		val2 = (u32) (unsigned long) utime;

	return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
}

$ sed -n '/get_timespec64(/,/;/p' linux/include/linux/time.h
int get_timespec64(struct timespec64 *ts,
		const struct __kernel_timespec __user *uts);

...

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---

Hello Thomas & Ingo,

I'm sorry I couldn't test the change in my computers,
as there is a bug since Linux 5.7 where I can't boot
(https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=974166).

Alex

 kernel/futex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 00259c7e288e..28577c7d2805 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3792,8 +3792,8 @@ long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 
 
 SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
-		struct __kernel_timespec __user *, utime, u32 __user *, uaddr2,
-		u32, val3)
+		const struct __kernel_timespec __user *, utime,
+		u32 __user *, uaddr2, u32, val3)
 {
 	struct timespec64 ts;
 	ktime_t t, *tp = NULL;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread