linux-kernel - Re: [PATCH v3 3/6] uapi: futex: Add a futex syscall

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: <CAK8P3a1sFSaMGhXkKJcLKgd68xSK3MMQuySXb62sWEacwzxt=g@mail.gmail.com>
Date:   Fri, 26 Nov 2021 08:24:27 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Alistair Francis <alistair.francis@...nsource.wdc.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Arnd Bergmann <arnd@...db.de>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Alistair Francis <alistair23@...il.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Ingo Molnar <mingo@...hat.com>,
        Darren Hart <dvhart@...radead.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        linux-perf-users@...r.kernel.org,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v3 3/6] uapi: futex: Add a futex syscall

On Fri, Nov 26, 2021 at 7:00 AM Alistair Francis
<alistair.francis@...nsource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@....com>
>
> This commit adds two futex syscall wrappers that are exposed to
> userspace.
>
> Neither the kernel or glibc currently expose a futex wrapper, so
> userspace is left performing raw syscalls. This has mostly been because
> the overloading of one of the arguments makes it impossible to provide a
> single type safe function.
>
> Until recently the single syscall has worked fine. With the introduction
> of a 64-bit time_t futex call on 32-bit architectures, this has become
> more complex. The logic of handling the two possible futex syscalls is
> complex and often implemented incorrectly.
>
> This patch adds two futux syscall functions that correctly handle the

s/futux/futex/

> time_t complexity for userspace.
>
> This idea is based on previous discussions:
> https://lore.kernel.org/lkml/CAK8P3a3x_EyCiPDpMK54y=Rtm-Wb08ym2TNiuAZgXhYrThcWTw@mail.gmail.com/
>
> Signed-off-by: Alistair Francis <alistair.francis@....com>

The implementation looks correct to me, the calling conventions seem
"good enough"
to me, but I wouldn't object if someone else can suggest a better interface.

One minor detail:

> +/**
> + * futex_syscall_timeout() - __NR_futex/__NR_futex_time64 syscall wrapper
> + * @uaddr:  address of first futex
> + * @op:   futex op code
> + * @val:  typically expected value of uaddr, but varies by op
> + * @timeout:  an absolute struct timespec
> + * @uaddr2: address of second futex for some ops
> + * @val3: varies by op
> + */
> +static inline int
> +__kernel_futex_syscall_timeout(volatile uint32_t *uaddr, int op, uint32_t val,
> +                     struct timespec *timeout, volatile uint32_t *uaddr2, int val3)

The two function names do not match the respective documentation.

        Arnd