Re: [PATCH v1 02/14] futex: Extend the FUTEX2 flags
From: Thomas Gleixner <hidden>
Date: 2023-07-31 21:14:20
Also in:
linux-arch, linux-mm, lkml
On Mon, Jul 31 2023 at 21:20, Peter Zijlstra wrote:
-#define FUTEX2_MASK (FUTEX2_64 | FUTEX2_PRIVATE) +#define FUTEX2_MASK (FUTEX2_SIZE_MASK | FUTEX2_PRIVATE)
Along with some comment which documents that the size "flags" constitute a number field and not flags in the sense of binary flags. And please name these size constants so it really becomes obvious: #define FUTEX2_SIZE_U32 2
quoted hunk ↗ jump to hunk
/** * futex_parse_waitv - Parse a waitv array from userspace@@ -208,11 +208,11 @@ static int futex_parse_waitv(struct fute return -EINVAL; if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall()) { - if ((aux.flags & FUTEX2_64) == FUTEX2_64) + if ((aux.flags & FUTEX2_SIZE_MASK) == FUTEX2_64) return -EINVAL; }
That should be part of the actual 64bit futex enablement, no?
- if ((aux.flags & FUTEX2_64) != FUTEX2_32) + if ((aux.flags & FUTEX2_SIZE_MASK) != FUTEX2_32) return -EINVAL;
In hindsight I think it was as mistake just to have this __u32 flags field in the new interface. Soemthing like the incomplete below might be retrofittable, no?
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h@@ -74,7 +74,12 @@ struct futex_waitv { __u64 val; __u64 uaddr; - __u32 flags; + union { + __u32 flags; + __u32 size : 2, + : 5, + private : 1; + }; __u32 __reserved; }; --- a/kernel/futex/syscalls.c +++ b/kernel/futex/syscalls.c
@@ -204,10 +204,10 @@ static int futex_parse_waitv(struct fute if (copy_from_user(&aux, &uwaitv[i], sizeof(aux))) return -EFAULT; - if ((aux.flags & ~FUTEX2_MASK) || aux.__reserved) + if ((aux.flags & ~FUTEX2_VALID_FLAGBITS) || aux.__reserved) return -EINVAL; - if (!(aux.flags & FUTEX2_32)) + if (aux.size != FUTEX2_SIZE_U32) return -EINVAL; futexv[i].w.flags = aux.flags;
If this muck already confused me right now, then I don't want to
experience the confusion factor 6 month down the road :)
Thanks,
tglx