Re: [PATCH v2 05/14] futex: Add sys_futex_wake()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Em 10/08/2023 09:13, Peter Zijlstra escreveu:
On Wed, Aug 09, 2023 at 07:25:19PM -0300, André Almeida wrote:
Hi Peter,

Em 07/08/2023 09:18, Peter Zijlstra escreveu:
To complement sys_futex_waitv() add sys_futex_wake(). This syscall
implements what was previously known as FUTEX_WAKE_BITSET except it
uses 'unsigned long' for the bitmask and takes FUTEX2 flags.

The 'unsigned long' allows FUTEX2_SIZE_U64 on 64bit platforms.

Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
Acked-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
---

[...]

+/*
+ * sys_futex_wake - Wake a number of futexes
+ * @uaddr:	Address of the futex(es) to wake
+ * @mask:	bitmask
+ * @nr:		Number of the futexes to wake
+ * @flags:	FUTEX2 flags
+ *
+ * Identical to the traditional FUTEX_WAKE_BITSET op, except it is part of the
+ * futex2 family of calls.
+ */
+
+SYSCALL_DEFINE4(futex_wake,
+		void __user *, uaddr,
+		unsigned long, mask,
+		int, nr,
+		unsigned int, flags)
+{

Do you think we could have a

	if (!nr)
		return 0;

here? Otherwise, calling futex_wake(&f, 0, flags) will wake 1 futex (if
available), which is a strange undocumented behavior in my opinion.

Oh 'cute' that.. yeah, but how about I put it ...

+	if (flags & ~FUTEX2_VALID_MASK)
+		return -EINVAL;
+
+	flags = futex2_to_flags(flags);
+	if (!futex_flags_valid(flags))
+		return -EINVAL;
+
+	if (!futex_validate_input(flags, mask))
+		return -EINVAL;

here, because otherwise we get:

	sys_futex_wake(&f, 0xFFFF, 0, FUTEX2_SIZE_U8)

to return 0, even though that is 'obviously' nonsensical and should
return -EINVAL. Or even garbage flags would be 'accepted'.

(because 0xFFFF is larger than U8 can accomodate)


That make sense to me, but we would also want to validate the value of f, if it's NULL or something strange to return -EINVAL... but this happens only inside get_futex_key()...

To make this right, I think we would need to move this verification to the syscall validation part:

	if (unlikely((address % sizeof(u32)) != 0))
		return -EINVAL;

	if (unlikely(!access_ok(uaddr, sizeof(u32))))
		return -EFAULT;

And have u32 replaced with the proper size being used.

+
+	return futex_wake(uaddr, flags, nr, mask);
+}



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux