* Re: [PATCH RFC v3 02/10] sched_getattr: port to copy_struct_to_user
From: Christian Brauner @ 2024-12-11 10:23 UTC (permalink / raw)
To: Florian Weimer, Aleksa Sarai, Ingo Molnar
Cc: Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Alexander Viro, Jan Kara, Arnd Bergmann, Shuah Khan, Kees Cook,
Mark Rutland, linux-kernel, linux-api, linux-fsdevel, linux-arch,
linux-kselftest, libc-alpha
In-Reply-To: <87y10nz9qo.fsf@oldenburg.str.redhat.com>
On Tue, Dec 10, 2024 at 07:14:07PM +0100, Florian Weimer wrote:
> * Aleksa Sarai:
>
> > sched_getattr(2) doesn't care about trailing non-zero bytes in the
> > (ksize > usize) case, so just use copy_struct_to_user() without checking
> > ignored_trailing.
>
> I think this is what causes glibc's misc/tst-sched_setattr test to fail
> on recent kernels. The previous non-modifying behavior was documented
> in the manual page:
>
> If the caller-provided attr buffer is larger than the kernel's
> sched_attr structure, the additional bytes in the user-space
> structure are not touched.
>
> I can just drop this part of the test if the kernel deems both behaviors
> valid.
I think in general both behaviors are valid but I would consider zeroing
the unknown parts of the provided buffer to be the safer option. And all
newer extensible struct system calls do that.
But if sched_getattr(2) wants to keep its old behavior it wouldn't be a
problem to just handle this case:
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 0d71fcbaf1e3..46140ec449ba 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -1126,6 +1126,15 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
}
kattr.size = min(usize, sizeof(kattr));
+ /*
+ * If userspace passed a larger structure than the kernel knows
+ * we historically didn't zero the unknown bits but
+ * copy_struct_to_user() will. Retain the old behavior by
+ * limiting the copy_to_user() to the size the kernel knows
+ * about.
+ */
+ if (usize > sizeof(kattr))
+ usize = sizeof(kattr);
return copy_struct_to_user(uattr, usize, &kattr, sizeof(kattr), NULL);
}
^ permalink raw reply related
* Re: [PATCH v22 2/8] security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits
From: Jeff Xu @ 2024-12-11 6:12 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Jeff Xu, Al Viro, Christian Brauner, Kees Cook, Paul Moore,
Serge Hallyn, Adhemerval Zanella Netto, Alejandro Colomar,
Aleksa Sarai, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
Casey Schaufler, Christian Heimes, Dmitry Vyukov, Elliott Hughes,
Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Roberto Sassu, Scott Shell, Shuah Khan, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Theodore Ts'o,
Thibaut Sautereau, Vincent Strubel, Xiaoming Ni, Yin Fengwei,
kernel-hardening, linux-api, linux-fsdevel, linux-integrity,
linux-kernel, linux-security-module, Andy Lutomirski
In-Reply-To: <20241210.FahfahPu5dae@digikod.net>
On Tue, Dec 10, 2024 at 8:48 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Thu, Dec 05, 2024 at 05:09:19PM +0100, Mickaël Salaün wrote:
> > The new SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE, and
> > their *_LOCKED counterparts are designed to be set by processes setting
> > up an execution environment, such as a user session, a container, or a
> > security sandbox. Unlike other securebits, these ones can be set by
> > unprivileged processes. Like seccomp filters or Landlock domains, the
> > securebits are inherited across processes.
> >
> > When SECBIT_EXEC_RESTRICT_FILE is set, programs interpreting code should
> > control executable resources according to execveat(2) + AT_EXECVE_CHECK
> > (see previous commit).
> >
> > When SECBIT_EXEC_DENY_INTERACTIVE is set, a process should deny
> > execution of user interactive commands (which excludes executable
> > regular files).
> >
> > Being able to configure each of these securebits enables system
> > administrators or owner of image containers to gradually validate the
> > related changes and to identify potential issues (e.g. with interpreter
> > or audit logs).
> >
> > It should be noted that unlike other security bits, the
> > SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE bits are
> > dedicated to user space willing to restrict itself. Because of that,
> > they only make sense in the context of a trusted environment (e.g.
> > sandbox, container, user session, full system) where the process
> > changing its behavior (according to these bits) and all its parent
> > processes are trusted. Otherwise, any parent process could just execute
> > its own malicious code (interpreting a script or not), or even enforce a
> > seccomp filter to mask these bits.
> >
> > Such a secure environment can be achieved with an appropriate access
> > control (e.g. mount's noexec option, file access rights, LSM policy) and
> > an enlighten ld.so checking that libraries are allowed for execution
> > e.g., to protect against illegitimate use of LD_PRELOAD.
> >
> > Ptrace restrictions according to these securebits would not make sense
> > because of the processes' trust assumption.
> >
> > Scripts may need some changes to deal with untrusted data (e.g. stdin,
> > environment variables), but that is outside the scope of the kernel.
> >
> > See chromeOS's documentation about script execution control and the
> > related threat model:
> > https://www.chromium.org/chromium-os/developer-library/guides/security/noexec-shell-scripts/
> >
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241205160925.230119-3-mic@digikod.net
> > ---
> >
> > Changes since v21:
> > * Extend user documentation with exception regarding tailored execution
> > environments (e.g. chromeOS's libc) as discussed with Jeff.
> >
> > Changes since v20:
> > * Move UAPI documentation to a dedicated RST file and format it.
> >
> > Changes since v19:
> > * Replace SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT with
> > SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE:
> > https://lore.kernel.org/all/20240710.eiKohpa4Phai@digikod.net/
> > * Remove the ptrace restrictions, suggested by Andy.
> > * Improve documentation according to the discussion with Jeff.
> >
> > New design since v18:
> > https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
> > ---
> > Documentation/userspace-api/check_exec.rst | 107 +++++++++++++++++++++
> > include/uapi/linux/securebits.h | 24 ++++-
> > security/commoncap.c | 29 ++++--
> > 3 files changed, 153 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
> > index 393dd7ca19c4..05dfe3b56f71 100644
> > --- a/Documentation/userspace-api/check_exec.rst
> > +++ b/Documentation/userspace-api/check_exec.rst
> > @@ -5,6 +5,31 @@
> > Executability check
> > ===================
> >
> > +The ``AT_EXECVE_CHECK`` :manpage:`execveat(2)` flag, and the
> > +``SECBIT_EXEC_RESTRICT_FILE`` and ``SECBIT_EXEC_DENY_INTERACTIVE`` securebits
> > +are intended for script interpreters and dynamic linkers to enforce a
> > +consistent execution security policy handled by the kernel. See the
> > +`samples/check-exec/inc.c`_ example.
> > +
> > +Whether an interpreter should check these securebits or not depends on the
> > +security risk of running malicious scripts with respect to the execution
> > +environment, and whether the kernel can check if a script is trustworthy or
> > +not. For instance, Python scripts running on a server can use arbitrary
> > +syscalls and access arbitrary files. Such interpreters should then be
> > +enlighten to use these securebits and let users define their security policy.
> > +However, a JavaScript engine running in a web browser should already be
> > +sandboxed and then should not be able to harm the user's environment.
> > +
> > +Script interpreters or dynamic linkers built for tailored execution environments
> > +(e.g. hardened Linux distributions or hermetic container images) could use
> > +``AT_EXECVE_CHECK`` without checking the related securebits if backward
> > +compatibility is handled by something else (e.g. atomic update ensuring that
> > +all legitimate libraries are allowed to be executed). It is then recommended
> > +for script interpreters and dynamic linkers to check the securebits at run time
> > +by default, but also to provide the ability for custom builds to behave like if
> > +``SECBIT_EXEC_RESTRICT_FILE`` or ``SECBIT_EXEC_DENY_INTERACTIVE`` were always
> > +set to 1 (i.e. always enforce restrictions).
>
> Jeff, does this work for you?
>
Yes. Thanks for updating this section.
> I'll update the IMA patch with a last version but otherwise it should be
> good: https://lore.kernel.org/all/20241210.Wie6ion7Aich@digikod.net/
>
> > +
> > AT_EXECVE_CHECK
> > ===============
> >
> > @@ -35,3 +60,85 @@ be executable, which also requires integrity guarantees.
> > To avoid race conditions leading to time-of-check to time-of-use issues,
> > ``AT_EXECVE_CHECK`` should be used with ``AT_EMPTY_PATH`` to check against a
> > file descriptor instead of a path.
> > +
> > +SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE
> > +==========================================================
> > +
> > +When ``SECBIT_EXEC_RESTRICT_FILE`` is set, a process should only interpret or
> > +execute a file if a call to :manpage:`execveat(2)` with the related file
> > +descriptor and the ``AT_EXECVE_CHECK`` flag succeed.
> > +
> > +This secure bit may be set by user session managers, service managers,
> > +container runtimes, sandboxer tools... Except for test environments, the
> > +related ``SECBIT_EXEC_RESTRICT_FILE_LOCKED`` bit should also be set.
> > +
> > +Programs should only enforce consistent restrictions according to the
> > +securebits but without relying on any other user-controlled configuration.
> > +Indeed, the use case for these securebits is to only trust executable code
> > +vetted by the system configuration (through the kernel), so we should be
> > +careful to not let untrusted users control this configuration.
> > +
> > +However, script interpreters may still use user configuration such as
> > +environment variables as long as it is not a way to disable the securebits
> > +checks. For instance, the ``PATH`` and ``LD_PRELOAD`` variables can be set by
> > +a script's caller. Changing these variables may lead to unintended code
> > +executions, but only from vetted executable programs, which is OK. For this to
> > +make sense, the system should provide a consistent security policy to avoid
> > +arbitrary code execution e.g., by enforcing a write xor execute policy.
> > +
> > +When ``SECBIT_EXEC_DENY_INTERACTIVE`` is set, a process should never interpret
> > +interactive user commands (e.g. scripts). However, if such commands are passed
> > +through a file descriptor (e.g. stdin), its content should be interpreted if a
> > +call to :manpage:`execveat(2)` with the related file descriptor and the
> > +``AT_EXECVE_CHECK`` flag succeed.
> > +
> > +For instance, script interpreters called with a script snippet as argument
> > +should always deny such execution if ``SECBIT_EXEC_DENY_INTERACTIVE`` is set.
> > +
> > +This secure bit may be set by user session managers, service managers,
> > +container runtimes, sandboxer tools... Except for test environments, the
> > +related ``SECBIT_EXEC_DENY_INTERACTIVE_LOCKED`` bit should also be set.
> > +
> > +Here is the expected behavior for a script interpreter according to combination
> > +of any exec securebits:
> > +
> > +1. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
> > +
> > + Always interpret scripts, and allow arbitrary user commands (default).
> > +
> > + No threat, everyone and everything is trusted, but we can get ahead of
> > + potential issues thanks to the call to :manpage:`execveat(2)` with
> > + ``AT_EXECVE_CHECK`` which should always be performed but ignored by the
> > + script interpreter. Indeed, this check is still important to enable systems
> > + administrators to verify requests (e.g. with audit) and prepare for
> > + migration to a secure mode.
> > +
> > +2. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
> > +
> > + Deny script interpretation if they are not executable, but allow
> > + arbitrary user commands.
> > +
> > + The threat is (potential) malicious scripts run by trusted (and not fooled)
> > + users. That can protect against unintended script executions (e.g. ``sh
> > + /tmp/*.sh``). This makes sense for (semi-restricted) user sessions.
> > +
> > +3. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
> > +
> > + Always interpret scripts, but deny arbitrary user commands.
> > +
> > + This use case may be useful for secure services (i.e. without interactive
> > + user session) where scripts' integrity is verified (e.g. with IMA/EVM or
> > + dm-verity/IPE) but where access rights might not be ready yet. Indeed,
> > + arbitrary interactive commands would be much more difficult to check.
> > +
> > +4. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
> > +
> > + Deny script interpretation if they are not executable, and also deny
> > + any arbitrary user commands.
> > +
> > + The threat is malicious scripts run by untrusted users (but trusted code).
> > + This makes sense for system services that may only execute trusted scripts.
> > +
> > +.. Links
> > +.. _samples/check-exec/inc.c:
> > + https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/check-exec/inc.c
>
Reviewed-by: Jeff Xu < jeffxu@chromium.org>
Tested-by: Jeff Xu <jeffxu@chromium.org>
^ permalink raw reply
* Re: [PATCH v22 1/8] exec: Add a new AT_EXECVE_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-12-11 5:58 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Roberto Sassu, Scott Shell,
Shuah Khan, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241205160925.230119-2-mic@digikod.net>
On Thu, Dec 5, 2024 at 8:10 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Add a new AT_EXECVE_CHECK flag to execveat(2) to check if a file would
> be allowed for execution. The main use case is for script interpreters
> and dynamic linkers to check execution permission according to the
> kernel's security policy. Another use case is to add context to access
> logs e.g., which script (instead of interpreter) accessed a file. As
> any executable code, scripts could also use this check [1].
>
> This is different from faccessat(2) + X_OK which only checks a subset of
> access rights (i.e. inode permission and mount options for regular
> files), but not the full context (e.g. all LSM access checks). The main
> use case for access(2) is for SUID processes to (partially) check access
> on behalf of their caller. The main use case for execveat(2) +
> AT_EXECVE_CHECK is to check if a script execution would be allowed,
> according to all the different restrictions in place. Because the use
> of AT_EXECVE_CHECK follows the exact kernel semantic as for a real
> execution, user space gets the same error codes.
>
> An interesting point of using execveat(2) instead of openat2(2) is that
> it decouples the check from the enforcement. Indeed, the security check
> can be logged (e.g. with audit) without blocking an execution
> environment not yet ready to enforce a strict security policy.
>
> LSMs can control or log execution requests with
> security_bprm_creds_for_exec(). However, to enforce a consistent and
> complete access control (e.g. on binary's dependencies) LSMs should
> restrict file executability, or measure executed files, with
> security_file_open() by checking file->f_flags & __FMODE_EXEC.
>
> Because AT_EXECVE_CHECK is dedicated to user space interpreters, it
> doesn't make sense for the kernel to parse the checked files, look for
> interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
> if the format is unknown. Because of that, security_bprm_check() is
> never called when AT_EXECVE_CHECK is used.
>
> It should be noted that script interpreters cannot directly use
> execveat(2) (without this new AT_EXECVE_CHECK flag) because this could
> lead to unexpected behaviors e.g., `python script.sh` could lead to Bash
> being executed to interpret the script. Unlike the kernel, script
> interpreters may just interpret the shebang as a simple comment, which
> should not change for backward compatibility reasons.
>
> Because scripts or libraries files might not currently have the
> executable permission set, or because we might want specific users to be
> allowed to run arbitrary scripts, the following patch provides a dynamic
> configuration mechanism with the SECBIT_EXEC_RESTRICT_FILE and
> SECBIT_EXEC_DENY_INTERACTIVE securebits.
>
> This is a redesign of the CLIP OS 4's O_MAYEXEC:
> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> This patch has been used for more than a decade with customized script
> interpreters. Some examples can be found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Acked-by: Paul Moore <paul@paul-moore.com>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> Link: https://docs.python.org/3/library/io.html#io.open_code [1]
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241205160925.230119-2-mic@digikod.net
> ---
>
> Changes since v21:
> * Remove the audit changes, requested by Paul.
> * Add Acked-by: Paul Moore.
> * Fix a typo in comments and in commit message.
> * Add SPDX-License-Identifier header to the documentation.
> * Rebase on v6.13-rc1 .
>
> Changes since v20:
> * Rename AT_CHECK to AT_EXECVE_CHECK, requested by Amir Goldstein and
> Serge Hallyn.
> * Move the UAPI documentation to a dedicated RST file.
> * Add Reviewed-by: Serge Hallyn
>
> Changes since v19:
> * Remove mention of "role transition" as suggested by Andy.
> * Highlight the difference between security_bprm_creds_for_exec() and
> the __FMODE_EXEC check for LSMs (in commit message and LSM's hooks) as
> discussed with Jeff.
> * Improve documentation both in UAPI comments and kernel comments
> (requested by Kees).
>
> New design since v18:
> https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
> ---
> Documentation/userspace-api/check_exec.rst | 37 ++++++++++++++++++++++
> Documentation/userspace-api/index.rst | 1 +
> fs/exec.c | 20 ++++++++++--
> include/linux/binfmts.h | 7 +++-
> include/uapi/linux/fcntl.h | 4 +++
> security/security.c | 10 ++++++
> 6 files changed, 76 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/userspace-api/check_exec.rst
>
> diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
> new file mode 100644
> index 000000000000..393dd7ca19c4
> --- /dev/null
> +++ b/Documentation/userspace-api/check_exec.rst
> @@ -0,0 +1,37 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. Copyright © 2024 Microsoft Corporation
> +
> +===================
> +Executability check
> +===================
> +
> +AT_EXECVE_CHECK
> +===============
> +
> +Passing the ``AT_EXECVE_CHECK`` flag to :manpage:`execveat(2)` only performs a
> +check on a regular file and returns 0 if execution of this file would be
> +allowed, ignoring the file format and then the related interpreter dependencies
> +(e.g. ELF libraries, script's shebang).
> +
> +Programs should always perform this check to apply kernel-level checks against
> +files that are not directly executed by the kernel but passed to a user space
> +interpreter instead. All files that contain executable code, from the point of
> +view of the interpreter, should be checked. However the result of this check
> +should only be enforced according to ``SECBIT_EXEC_RESTRICT_FILE`` or
> +``SECBIT_EXEC_DENY_INTERACTIVE.``.
> +
> +The main purpose of this flag is to improve the security and consistency of an
> +execution environment to ensure that direct file execution (e.g.
> +``./script.sh``) and indirect file execution (e.g. ``sh script.sh``) lead to
> +the same result. For instance, this can be used to check if a file is
> +trustworthy according to the caller's environment.
> +
> +In a secure environment, libraries and any executable dependencies should also
> +be checked. For instance, dynamic linking should make sure that all libraries
> +are allowed for execution to avoid trivial bypass (e.g. using ``LD_PRELOAD``).
> +For such secure execution environment to make sense, only trusted code should
> +be executable, which also requires integrity guarantees.
> +
> +To avoid race conditions leading to time-of-check to time-of-use issues,
> +``AT_EXECVE_CHECK`` should be used with ``AT_EMPTY_PATH`` to check against a
> +file descriptor instead of a path.
> diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
> index 274cc7546efc..6272bcf11296 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -35,6 +35,7 @@ Security-related interfaces
> mfd_noexec
> spec_ctrl
> tee
> + check_exec
>
> Devices and I/O
> ===============
> diff --git a/fs/exec.c b/fs/exec.c
> index 98cb7ba9983c..e3f461096e84 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -892,7 +892,8 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
> .lookup_flags = LOOKUP_FOLLOW,
> };
>
> - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + if ((flags &
> + ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_EXECVE_CHECK)) != 0)
> return ERR_PTR(-EINVAL);
> if (flags & AT_SYMLINK_NOFOLLOW)
> open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
> @@ -1541,6 +1542,21 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
> }
> bprm->interp = bprm->filename;
>
> + /*
> + * At this point, security_file_open() has already been called (with
> + * __FMODE_EXEC) and access control checks for AT_EXECVE_CHECK will
> + * stop just after the security_bprm_creds_for_exec() call in
> + * bprm_execve(). Indeed, the kernel should not try to parse the
> + * content of the file with exec_binprm() nor change the calling
> + * thread, which means that the following security functions will not
> + * be called:
> + * - security_bprm_check()
> + * - security_bprm_creds_from_file()
> + * - security_bprm_committing_creds()
> + * - security_bprm_committed_creds()
> + */
> + bprm->is_check = !!(flags & AT_EXECVE_CHECK);
> +
> retval = bprm_mm_init(bprm);
> if (!retval)
> return bprm;
> @@ -1836,7 +1852,7 @@ static int bprm_execve(struct linux_binprm *bprm)
>
> /* Set the unchanging part of bprm->cred */
> retval = security_bprm_creds_for_exec(bprm);
> - if (retval)
> + if (retval || bprm->is_check)
> goto out;
>
> retval = exec_binprm(bprm);
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index e6c00e860951..8ff0eb3644a1 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -42,7 +42,12 @@ struct linux_binprm {
> * Set when errors can no longer be returned to the
> * original userspace.
> */
> - point_of_no_return:1;
> + point_of_no_return:1,
> + /*
> + * Set by user space to check executability according to the
> + * caller's environment.
> + */
> + is_check:1;
> struct file *executable; /* Executable to pass to the interpreter */
> struct file *interpreter;
> struct file *file;
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 6e6907e63bfc..a15ac2fa4b20 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -155,4 +155,8 @@
> #define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
> #define AT_HANDLE_CONNECTABLE 0x002 /* Request a connectable file handle */
>
> +/* Flags for execveat2(2). */
> +#define AT_EXECVE_CHECK 0x10000 /* Only perform a check if execution
> + would be allowed. */
> +
> #endif /* _UAPI_LINUX_FCNTL_H */
> diff --git a/security/security.c b/security/security.c
> index 09664e09fec9..dae7e903947f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1248,6 +1248,12 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
> * to 1 if AT_SECURE should be set to request libc enable secure mode. @bprm
> * contains the linux_binprm structure.
> *
> + * If execveat(2) is called with the AT_EXECVE_CHECK flag, bprm->is_check is
> + * set. The result must be the same as without this flag even if the execution
> + * will never really happen and @bprm will always be dropped.
> + *
> + * This hook must not change current->cred, only @bprm->cred.
> + *
> * Return: Returns 0 if the hook is successful and permission is granted.
> */
> int security_bprm_creds_for_exec(struct linux_binprm *bprm)
> @@ -3098,6 +3104,10 @@ int security_file_receive(struct file *file)
> * Save open-time permission checking state for later use upon file_permission,
> * and recheck access if anything has changed since inode_permission.
> *
> + * We can check if a file is opened for execution (e.g. execve(2) call), either
> + * directly or indirectly (e.g. ELF's ld.so) by checking file->f_flags &
> + * __FMODE_EXEC .
> + *
> * Return: Returns 0 if permission is granted.
> */
> int security_file_open(struct file *file)
> --
> 2.47.1
>
Reviewed-by: Jeff Xu < jeffxu@chromium.org>
Tested-by: Jeff Xu <jeffxu@chromium.org>
>
^ permalink raw reply
* Re: [PATCH v22 0/8] Script execution control (was O_MAYEXEC)
From: jeffxu @ 2024-12-11 5:47 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Roberto Sassu, Scott Shell,
Shuah Khan, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241205160925.230119-1-mic@digikod.net>
On Thu, Dec 5, 2024 at 8:09 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Hi,
>
> The goal of this patch series is to be able to ensure that direct file
> execution (e.g. ./script.sh) and indirect file execution (e.g. sh
> script.sh) lead to the same result, especially from a security point of
> view.
>
> The main changes from the previous version are the IMA patch to properly
> log access check requests with audit, removal of audit change, an
> extended documentation for tailored distros, a rebase on v6.13-rc1, and
> some minor cosmetic changes.
>
> The current status is summarized in this article:
> https://lwn.net/Articles/982085/
> I also gave a talk at LPC last month:
> https://lpc.events/event/18/contributions/1692/
> And here is a proof of concept for Python (for now, for the previous
> version: v19): https://github.com/zooba/spython/pull/12
>
> Kees, would you like to take this series in your tree?
>
> Overview
> --------
>
> This patch series is a new approach of the initial O_MAYEXEC feature,
> and a revamp of the previous patch series. Taking into account the last
> reviews [1], we now stick to the kernel semantic for file executability.
> One major change is the clear split between access check and policy
> management.
>
> The first patch brings the AT_EXECVE_CHECK flag to execveat(2). The
> goal is to enable user space to check if a file could be executed (by
> the kernel). Unlike stat(2) that only checks file permissions,
> execveat2(2) + AT_EXECVE_CHECK take into account the full context,
> including mount points (noexec), caller's limits, and all potential LSM
> extra checks (e.g. argv, envp, credentials).
>
> The second patch brings two new securebits used to set or get a security
> policy for a set of processes. For this to be meaningful, all
> executable code needs to be trusted. In practice, this means that
> (malicious) users can be restricted to only run scripts provided (and
> trusted) by the system.
>
> [1] https://lore.kernel.org/r/CAHk-=wjPGNLyzeBMWdQu+kUdQLHQugznwY7CvWjmvNW47D5sog@mail.gmail.com
>
> Script execution
> ----------------
>
> One important thing to keep in mind is that the goal of this patch
> series is to get the same security restrictions with these commands:
> * ./script.py
> * python script.py
> * python < script.py
> * python -m script.py
>
> However, on secure systems, we should be able to forbid these commands
> because there is no way to reliably identify the origin of the script:
> * xargs -a script.py -d '\r' -- python -c
> * cat script.py | python
> * python
>
> Background
> ----------
>
> Compared to the previous patch series, there is no more dedicated
> syscall nor sysctl configuration. This new patch series only add new
> flags: one for execveat(2) and four for prctl(2).
>
> This kind of script interpreter restriction may already be used in
> hardened systems, which may need to fork interpreters and install
> different versions of the binaries. This mechanism should enable to
> avoid the use of duplicate binaries (and potential forked source code)
> for secure interpreters (e.g. secure Python [2]) by making it possible
> to dynamically enforce restrictions or not.
>
> The ability to control script execution is also required to close a
> major IMA measurement/appraisal interpreter integrity [3].
>
> This new execveat + AT_EXECVE_CHECK should not be confused with the
> O_EXEC flag (for open) which is intended for execute-only, which
> obviously doesn't work for scripts.
>
> I gave a talk about controlling script execution where I explain the
> previous approaches [4]. The design of the WIP RFC I talked about
> changed quite a bit since then.
>
> [2] https://github.com/zooba/spython
> [3] https://lore.kernel.org/lkml/20211014130125.6991-1-zohar@linux.ibm.com/
> [4] https://lssna2023.sched.com/event/1K7bO
>
> Execution policy
> ----------------
>
> The "execution" usage means that the content of the file descriptor is
> trusted according to the system policy to be executed by user space,
> which means that it interprets the content or (try to) maps it as
> executable memory.
>
> It is important to note that this can only enable to extend access
> control managed by the kernel. Hence it enables current access control
> mechanism to be extended and become a superset of what they can
> currently control. Indeed, the security policy could also be delegated
> to an LSM, either a MAC system or an integrity system.
>
> Complementary W^X protections can be brought by SELinux or IPE [5].
>
> Being able to restrict execution also enables to protect the kernel by
> restricting arbitrary syscalls that an attacker could perform with a
> crafted binary or certain script languages. It also improves multilevel
> isolation by reducing the ability of an attacker to use side channels
> with specific code. These restrictions can natively be enforced for ELF
> binaries (with the noexec mount option) but require this kernel
> extension to properly handle scripts (e.g. Python, Perl). To get a
> consistent execution policy, additional memory restrictions should also
> be enforced (e.g. thanks to SELinux).
>
> [5] https://lore.kernel.org/lkml/1716583609-21790-1-git-send-email-wufan@linux.microsoft.com/
>
> Prerequisite for security use
> -----------------------------
>
> Because scripts might not currently have the executable permission and
> still run well as is, or because we might want specific users to be
> allowed to run arbitrary scripts, we also need a configuration
> mechanism.
>
> According to the threat model, to get a secure execution environment on
> top of these changes, it might be required to configure and enable
> existing security mechanisms such as secure boot, restrictive mount
> points (e.g. with rw AND noexec), correct file permissions (including
> executable libraries), IMA/EVM, SELinux policy...
>
> The first thing to patch is the libc to check loaded libraries (e.g. see
> chromeOS changes). The second thing to patch are the script
> interpreters by checking direct scripts executability and by checking
> their own libraries (e.g. Python's imported files or argument-passed
> modules). For instance, the PEP 578 [6] (Runtime Audit Hooks) enables
> Python 3.8 to be extended with policy enforcement points related to code
> interpretation, which can be used to align with the PowerShell audit
> features. Additional Python security improvements (e.g. a limited
> interpreter without -c, stdin piping of code) are developed [2] [7].
>
> [6] https://www.python.org/dev/peps/pep-0578/
> [7] https://lore.kernel.org/lkml/0c70debd-e79e-d514-06c6-4cd1e021fa8b@python.org/
>
> libc patch
> ----------
>
> Dynamic linking needs still need to check the libraries the same way
> interpreters need to check scripts.
>
> chromeOS patches glibc with a fstatvfs check [8] [9]. This enables to
> check against noexec mount points, which is OK but doesn't fit with
> execve semantics. Moreover, the kernel is not aware of such check, so
> all access control checks are not performed (e.g. file permission, LSMs
> security policies, integrity and authenticity checks), it is not handled
> with audit, and more importantly this would not work on generic
> distributions because of the strict requirement and chromeOS-specific
> assumptions.
>
Indeed, ChromeOS currently carries a private patch to work around this.
After this is accepted by the kernel and glibc's dynamic linker,
ChromeOS will use this and remove the need of carrying a private patch
from release to release.
Thanks !
-Jeff
> [8] https://issuetracker.google.com/issues/40054993
> [9] https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6abfc9e327241a5f684b8b941c899b7ca8b6dbc1/sys-libs/glibc/files/local/glibc-2.37/0007-Deny-LD_PRELOAD-of-files-in-NOEXEC-mount.patch
>
> Examples
> --------
>
> The initial idea comes from CLIP OS 4 and the original implementation
> has been used for more than a decade:
> https://github.com/clipos-archive/clipos4_doc
> Chrome OS has a similar approach:
> https://www.chromium.org/chromium-os/developer-library/guides/security/noexec-shell-scripts/
>
> User space patches can be found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> There is more than the O_MAYEXEC changes (which matches this search)
> e.g., to prevent Python interactive execution. There are patches for
> Bash, Wine, Java (Icedtea), Busybox's ash, Perl and Python. There are
> also some related patches which do not directly rely on O_MAYEXEC but
> which restrict the use of browser plugins and extensions, which may be
> seen as scripts too:
> https://github.com/clipos-archive/clipos4_portage-overlay/tree/master/www-client
>
> Past talks and articles
> -----------------------
>
> Closing the script execution control gap at Linux Plumbers Conference
> 2024: https://lpc.events/event/18/contributions/1692/
>
> An introduction to O_MAYEXEC was given at the Linux Security Summit
> Europe 2018 - Linux Kernel Security Contributions by ANSSI:
> https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
>
> The "write xor execute" principle was explained at Kernel Recipes 2018 -
> CLIP OS: a defense-in-depth OS:
> https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s
>
> LWN articles:
> * https://lwn.net/Articles/982085/
> * https://lwn.net/Articles/832959/
> * https://lwn.net/Articles/820000/
>
> FAQ
> Link: https://lore.kernel.org/r/20241205160925.230119-1-mic@digikod.net
> ---
>
> Q: Why not extend open(2) or openat2(2) with a new flag like O_MAYEXEC?
> A: Because it is not flexible enough:
> https://lore.kernel.org/r/CAG48ez0NAV5gPgmbDaSjo=zzE=FgnYz=-OHuXwu0Vts=B5gesA@mail.gmail.com
>
> Q: Why not only allowing file descriptor to avoid TOCTOU?
> A: Because there are different use cases:
> https://lore.kernel.org/r/CAHk-=whb=XuU=LGKnJWaa7LOYQz9VwHs8SLfgLbT5sf2VAbX1A@mail.gmail.com
>
> Q: We can copy a script into a memfd and use it as an executable FD.
> Wouldn't that bypass the purpose of this patch series?
> A: If an attacker can create a memfd it means that a
> malicious/compromised code is already running and it's too late for
> script execution control to help. This patch series makes it more
> difficult for an attacker to execute arbitrary code on a trusted
> system in the first place:
> https://lore.kernel.org/all/20240717.AGh2shahc9ee@digikod.net/
>
> Q: What about ROP?
> A: See previous answer. If ROP is exploited then the attacker already
> controls some code:
> https://lore.kernel.org/all/20240718.ahph4che5Shi@digikod.net/
>
> Q: What about LD_PRELOAD environment variable?
> A: The dynamic linker should be enlighten to check if libraries are
> allowed to be loaded.
>
> Q: What about The PATH environment variable?
> A: All programs allowed to be executed are deemed trusted.
>
> Q: Should we check seccomp filters too?
> A: Yes, they should be considered as executable code because they can
> change the behavior of processes, similarly to code injection:
> https://lore.kernel.org/all/20240705.IeTheequ7Ooj@digikod.net/
>
> Q: Could that be used for role transition?
> A: That would be risky and difficult to implement correctly:
> https://lore.kernel.org/all/20240723.Tae5oovie2ah@digikod.net/
>
> Previous versions
> -----------------
>
> v20: https://lore.kernel.org/r/20241011184422.977903-1-mic@digikod.net
> v19: https://lore.kernel.org/r/20240704190137.696169-1-mic@digikod.net
> v18: https://lore.kernel.org/r/20220104155024.48023-1-mic@digikod.net
> v17: https://lore.kernel.org/r/20211115185304.198460-1-mic@digikod.net
> v16: https://lore.kernel.org/r/20211110190626.257017-1-mic@digikod.net
> v15: https://lore.kernel.org/r/20211012192410.2356090-1-mic@digikod.net
> v14: https://lore.kernel.org/r/20211008104840.1733385-1-mic@digikod.net
> v13: https://lore.kernel.org/r/20211007182321.872075-1-mic@digikod.net
> v12: https://lore.kernel.org/r/20201203173118.379271-1-mic@digikod.net
> v11: https://lore.kernel.org/r/20201019164932.1430614-1-mic@digikod.net
> v10: https://lore.kernel.org/r/20200924153228.387737-1-mic@digikod.net
> v9: https://lore.kernel.org/r/20200910164612.114215-1-mic@digikod.net
> v8: https://lore.kernel.org/r/20200908075956.1069018-1-mic@digikod.net
> v7: https://lore.kernel.org/r/20200723171227.446711-1-mic@digikod.net
> v6: https://lore.kernel.org/r/20200714181638.45751-1-mic@digikod.net
> v5: https://lore.kernel.org/r/20200505153156.925111-1-mic@digikod.net
> v4: https://lore.kernel.org/r/20200430132320.699508-1-mic@digikod.net
> v3: https://lore.kernel.org/r/20200428175129.634352-1-mic@digikod.net
> v2: https://lore.kernel.org/r/20190906152455.22757-1-mic@digikod.net
> v1: https://lore.kernel.org/r/20181212081712.32347-1-mic@digikod.net
>
> Regards,
>
> Mickaël Salaün (7):
> exec: Add a new AT_EXECVE_CHECK flag to execveat(2)
> security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits
> selftests/exec: Add 32 tests for AT_EXECVE_CHECK and exec securebits
> selftests/landlock: Add tests for execveat + AT_EXECVE_CHECK
> samples/check-exec: Add set-exec
> selftests: ktap_helpers: Fix uninitialized variable
> samples/check-exec: Add an enlighten "inc" interpreter and 28 tests
>
> Mimi Zohar (1):
> ima: instantiate the bprm_creds_for_exec() hook
>
> Documentation/userspace-api/check_exec.rst | 144 ++++++
> Documentation/userspace-api/index.rst | 1 +
> fs/exec.c | 20 +-
> include/linux/binfmts.h | 7 +-
> include/uapi/linux/audit.h | 1 +
> include/uapi/linux/fcntl.h | 4 +
> include/uapi/linux/securebits.h | 24 +-
> samples/Kconfig | 9 +
> samples/Makefile | 1 +
> samples/check-exec/.gitignore | 2 +
> samples/check-exec/Makefile | 15 +
> samples/check-exec/inc.c | 205 ++++++++
> samples/check-exec/run-script-ask.inc | 9 +
> samples/check-exec/script-ask.inc | 5 +
> samples/check-exec/script-exec.inc | 4 +
> samples/check-exec/script-noexec.inc | 4 +
> samples/check-exec/set-exec.c | 85 ++++
> security/commoncap.c | 29 +-
> security/integrity/ima/ima_appraise.c | 27 +-
> security/integrity/ima/ima_main.c | 29 ++
> security/security.c | 10 +
> tools/testing/selftests/exec/.gitignore | 4 +
> tools/testing/selftests/exec/Makefile | 19 +-
> .../selftests/exec/check-exec-tests.sh | 205 ++++++++
> tools/testing/selftests/exec/check-exec.c | 456 ++++++++++++++++++
> tools/testing/selftests/exec/config | 2 +
> tools/testing/selftests/exec/false.c | 5 +
> .../selftests/kselftest/ktap_helpers.sh | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 27 ++
> 29 files changed, 1341 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/userspace-api/check_exec.rst
> create mode 100644 samples/check-exec/.gitignore
> create mode 100644 samples/check-exec/Makefile
> create mode 100644 samples/check-exec/inc.c
> create mode 100755 samples/check-exec/run-script-ask.inc
> create mode 100755 samples/check-exec/script-ask.inc
> create mode 100755 samples/check-exec/script-exec.inc
> create mode 100644 samples/check-exec/script-noexec.inc
> create mode 100644 samples/check-exec/set-exec.c
> create mode 100755 tools/testing/selftests/exec/check-exec-tests.sh
> create mode 100644 tools/testing/selftests/exec/check-exec.c
> create mode 100644 tools/testing/selftests/exec/config
> create mode 100644 tools/testing/selftests/exec/false.c
>
>
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> --
> 2.47.1
>
>
^ permalink raw reply
* Re: [PATCH v22 8/8] ima: instantiate the bprm_creds_for_exec() hook
From: Paul Moore @ 2024-12-10 21:29 UTC (permalink / raw)
To: Mickaël Salaün, Mimi Zohar, Roberto Sassu
Cc: Al Viro, Christian Brauner, Kees Cook, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Nicolas Bouchinet, Scott Shell, Shuah Khan, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Theodore Ts'o,
Thibaut Sautereau, Vincent Strubel, Xiaoming Ni, Yin Fengwei,
kernel-hardening, linux-api, linux-fsdevel, linux-integrity,
linux-kernel, linux-security-module
In-Reply-To: <20241205160925.230119-9-mic@digikod.net>
On Thu, Dec 5, 2024 at 11:10 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> From: Mimi Zohar <zohar@linux.ibm.com>
>
> Like direct file execution (e.g. ./script.sh), indirect file execution
> (e.g. sh script.sh) needs to be measured and appraised. Instantiate
> the new security_bprm_creds_for_exec() hook to measure and verify the
> indirect file's integrity. Unlike direct file execution, indirect file
> execution is optionally enforced by the interpreter.
>
> Differentiate kernel and userspace enforced integrity audit messages.
>
> Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> Link: https://lore.kernel.org/r/20241204192514.40308-1-zohar@linux.ibm.com
> Reviewed-by: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241205160925.230119-9-mic@digikod.net
> ---
>
> I added both a Reviewed-by and a Signed-off-by because I may not be the
> committer.
>
> Changes since v21:
> * New patch cherry-picked from IMA's patch v3:
> https://lore.kernel.org/r/67b2e94f263bf9a0099efe74cce659d6acb16fe9.camel@linux.ibm.com
> * Fix a typo in comment: s/execvat/execveat/ .
> ---
> include/uapi/linux/audit.h | 1 +
> security/integrity/ima/ima_appraise.c | 27 +++++++++++++++++++++++--
> security/integrity/ima/ima_main.c | 29 +++++++++++++++++++++++++++
> 3 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 75e21a135483..826337905466 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -161,6 +161,7 @@
> #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
> #define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */
> #define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */
> +#define AUDIT_INTEGRITY_DATA_CHECK 1808 /* Userspace enforced data integrity */
FWIW, in the last discussion I believe Mimi preferred the name
AUDIT_INTEGRITY_USERSPACE.
https://lore.kernel.org/linux-security-module/b6dc4d8b23b822638ab676055809503060c0bca2.camel@linux.ibm.com
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC v3 02/10] sched_getattr: port to copy_struct_to_user
From: Florian Weimer @ 2024-12-10 18:14 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Alexander Viro, Christian Brauner, Jan Kara,
Arnd Bergmann, Shuah Khan, Kees Cook, Mark Rutland, linux-kernel,
linux-api, linux-fsdevel, linux-arch, linux-kselftest, libc-alpha
In-Reply-To: <20241010-extensible-structs-check_fields-v3-2-d2833dfe6edd@cyphar.com>
* Aleksa Sarai:
> sched_getattr(2) doesn't care about trailing non-zero bytes in the
> (ksize > usize) case, so just use copy_struct_to_user() without checking
> ignored_trailing.
I think this is what causes glibc's misc/tst-sched_setattr test to fail
on recent kernels. The previous non-modifying behavior was documented
in the manual page:
If the caller-provided attr buffer is larger than the kernel's
sched_attr structure, the additional bytes in the user-space
structure are not touched.
I can just drop this part of the test if the kernel deems both behaviors
valid.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH v22 2/8] security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits
From: Mickaël Salaün @ 2024-12-10 16:46 UTC (permalink / raw)
To: Jeff Xu
Cc: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Roberto Sassu, Scott Shell,
Shuah Khan, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Andy Lutomirski
In-Reply-To: <20241205160925.230119-3-mic@digikod.net>
On Thu, Dec 05, 2024 at 05:09:19PM +0100, Mickaël Salaün wrote:
> The new SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE, and
> their *_LOCKED counterparts are designed to be set by processes setting
> up an execution environment, such as a user session, a container, or a
> security sandbox. Unlike other securebits, these ones can be set by
> unprivileged processes. Like seccomp filters or Landlock domains, the
> securebits are inherited across processes.
>
> When SECBIT_EXEC_RESTRICT_FILE is set, programs interpreting code should
> control executable resources according to execveat(2) + AT_EXECVE_CHECK
> (see previous commit).
>
> When SECBIT_EXEC_DENY_INTERACTIVE is set, a process should deny
> execution of user interactive commands (which excludes executable
> regular files).
>
> Being able to configure each of these securebits enables system
> administrators or owner of image containers to gradually validate the
> related changes and to identify potential issues (e.g. with interpreter
> or audit logs).
>
> It should be noted that unlike other security bits, the
> SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE bits are
> dedicated to user space willing to restrict itself. Because of that,
> they only make sense in the context of a trusted environment (e.g.
> sandbox, container, user session, full system) where the process
> changing its behavior (according to these bits) and all its parent
> processes are trusted. Otherwise, any parent process could just execute
> its own malicious code (interpreting a script or not), or even enforce a
> seccomp filter to mask these bits.
>
> Such a secure environment can be achieved with an appropriate access
> control (e.g. mount's noexec option, file access rights, LSM policy) and
> an enlighten ld.so checking that libraries are allowed for execution
> e.g., to protect against illegitimate use of LD_PRELOAD.
>
> Ptrace restrictions according to these securebits would not make sense
> because of the processes' trust assumption.
>
> Scripts may need some changes to deal with untrusted data (e.g. stdin,
> environment variables), but that is outside the scope of the kernel.
>
> See chromeOS's documentation about script execution control and the
> related threat model:
> https://www.chromium.org/chromium-os/developer-library/guides/security/noexec-shell-scripts/
>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241205160925.230119-3-mic@digikod.net
> ---
>
> Changes since v21:
> * Extend user documentation with exception regarding tailored execution
> environments (e.g. chromeOS's libc) as discussed with Jeff.
>
> Changes since v20:
> * Move UAPI documentation to a dedicated RST file and format it.
>
> Changes since v19:
> * Replace SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT with
> SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE:
> https://lore.kernel.org/all/20240710.eiKohpa4Phai@digikod.net/
> * Remove the ptrace restrictions, suggested by Andy.
> * Improve documentation according to the discussion with Jeff.
>
> New design since v18:
> https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
> ---
> Documentation/userspace-api/check_exec.rst | 107 +++++++++++++++++++++
> include/uapi/linux/securebits.h | 24 ++++-
> security/commoncap.c | 29 ++++--
> 3 files changed, 153 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
> index 393dd7ca19c4..05dfe3b56f71 100644
> --- a/Documentation/userspace-api/check_exec.rst
> +++ b/Documentation/userspace-api/check_exec.rst
> @@ -5,6 +5,31 @@
> Executability check
> ===================
>
> +The ``AT_EXECVE_CHECK`` :manpage:`execveat(2)` flag, and the
> +``SECBIT_EXEC_RESTRICT_FILE`` and ``SECBIT_EXEC_DENY_INTERACTIVE`` securebits
> +are intended for script interpreters and dynamic linkers to enforce a
> +consistent execution security policy handled by the kernel. See the
> +`samples/check-exec/inc.c`_ example.
> +
> +Whether an interpreter should check these securebits or not depends on the
> +security risk of running malicious scripts with respect to the execution
> +environment, and whether the kernel can check if a script is trustworthy or
> +not. For instance, Python scripts running on a server can use arbitrary
> +syscalls and access arbitrary files. Such interpreters should then be
> +enlighten to use these securebits and let users define their security policy.
> +However, a JavaScript engine running in a web browser should already be
> +sandboxed and then should not be able to harm the user's environment.
> +
> +Script interpreters or dynamic linkers built for tailored execution environments
> +(e.g. hardened Linux distributions or hermetic container images) could use
> +``AT_EXECVE_CHECK`` without checking the related securebits if backward
> +compatibility is handled by something else (e.g. atomic update ensuring that
> +all legitimate libraries are allowed to be executed). It is then recommended
> +for script interpreters and dynamic linkers to check the securebits at run time
> +by default, but also to provide the ability for custom builds to behave like if
> +``SECBIT_EXEC_RESTRICT_FILE`` or ``SECBIT_EXEC_DENY_INTERACTIVE`` were always
> +set to 1 (i.e. always enforce restrictions).
Jeff, does this work for you?
I'll update the IMA patch with a last version but otherwise it should be
good: https://lore.kernel.org/all/20241210.Wie6ion7Aich@digikod.net/
> +
> AT_EXECVE_CHECK
> ===============
>
> @@ -35,3 +60,85 @@ be executable, which also requires integrity guarantees.
> To avoid race conditions leading to time-of-check to time-of-use issues,
> ``AT_EXECVE_CHECK`` should be used with ``AT_EMPTY_PATH`` to check against a
> file descriptor instead of a path.
> +
> +SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE
> +==========================================================
> +
> +When ``SECBIT_EXEC_RESTRICT_FILE`` is set, a process should only interpret or
> +execute a file if a call to :manpage:`execveat(2)` with the related file
> +descriptor and the ``AT_EXECVE_CHECK`` flag succeed.
> +
> +This secure bit may be set by user session managers, service managers,
> +container runtimes, sandboxer tools... Except for test environments, the
> +related ``SECBIT_EXEC_RESTRICT_FILE_LOCKED`` bit should also be set.
> +
> +Programs should only enforce consistent restrictions according to the
> +securebits but without relying on any other user-controlled configuration.
> +Indeed, the use case for these securebits is to only trust executable code
> +vetted by the system configuration (through the kernel), so we should be
> +careful to not let untrusted users control this configuration.
> +
> +However, script interpreters may still use user configuration such as
> +environment variables as long as it is not a way to disable the securebits
> +checks. For instance, the ``PATH`` and ``LD_PRELOAD`` variables can be set by
> +a script's caller. Changing these variables may lead to unintended code
> +executions, but only from vetted executable programs, which is OK. For this to
> +make sense, the system should provide a consistent security policy to avoid
> +arbitrary code execution e.g., by enforcing a write xor execute policy.
> +
> +When ``SECBIT_EXEC_DENY_INTERACTIVE`` is set, a process should never interpret
> +interactive user commands (e.g. scripts). However, if such commands are passed
> +through a file descriptor (e.g. stdin), its content should be interpreted if a
> +call to :manpage:`execveat(2)` with the related file descriptor and the
> +``AT_EXECVE_CHECK`` flag succeed.
> +
> +For instance, script interpreters called with a script snippet as argument
> +should always deny such execution if ``SECBIT_EXEC_DENY_INTERACTIVE`` is set.
> +
> +This secure bit may be set by user session managers, service managers,
> +container runtimes, sandboxer tools... Except for test environments, the
> +related ``SECBIT_EXEC_DENY_INTERACTIVE_LOCKED`` bit should also be set.
> +
> +Here is the expected behavior for a script interpreter according to combination
> +of any exec securebits:
> +
> +1. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
> +
> + Always interpret scripts, and allow arbitrary user commands (default).
> +
> + No threat, everyone and everything is trusted, but we can get ahead of
> + potential issues thanks to the call to :manpage:`execveat(2)` with
> + ``AT_EXECVE_CHECK`` which should always be performed but ignored by the
> + script interpreter. Indeed, this check is still important to enable systems
> + administrators to verify requests (e.g. with audit) and prepare for
> + migration to a secure mode.
> +
> +2. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
> +
> + Deny script interpretation if they are not executable, but allow
> + arbitrary user commands.
> +
> + The threat is (potential) malicious scripts run by trusted (and not fooled)
> + users. That can protect against unintended script executions (e.g. ``sh
> + /tmp/*.sh``). This makes sense for (semi-restricted) user sessions.
> +
> +3. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
> +
> + Always interpret scripts, but deny arbitrary user commands.
> +
> + This use case may be useful for secure services (i.e. without interactive
> + user session) where scripts' integrity is verified (e.g. with IMA/EVM or
> + dm-verity/IPE) but where access rights might not be ready yet. Indeed,
> + arbitrary interactive commands would be much more difficult to check.
> +
> +4. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
> +
> + Deny script interpretation if they are not executable, and also deny
> + any arbitrary user commands.
> +
> + The threat is malicious scripts run by untrusted users (but trusted code).
> + This makes sense for system services that may only execute trusted scripts.
> +
> +.. Links
> +.. _samples/check-exec/inc.c:
> + https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/check-exec/inc.c
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: D. Wythe @ 2024-12-10 6:13 UTC (permalink / raw)
To: John Ousterhout; +Cc: D. Wythe, netdev, linux-api
In-Reply-To: <CAGXJAmwA-aEdEWezOxWhHB8tdsB6aaBYjwYCo+=Hnhh0j8up4Q@mail.gmail.com>
On Mon, Dec 09, 2024 at 09:50:03PM -0800, John Ousterhout wrote:
It seems I misunderstood your point... I'm indeed not familiar with this.
Perhaps other members of the community can help you.
D. Wythe
> Thanks for the additional information. My concern was whether the
> available kernel virtual address space for vmapping is a scarce
> resource, in which case it might not be good for Homa to lock up large
> amounts of it for long periods of time. I'm not as worried about
> physical memory usage (that will happen regardless of whether the
> buffers get vmapped into the kernel).
>
> -John-
>
>
> On Mon, Dec 9, 2024 at 9:14 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> >
> > On Mon, Dec 09, 2024 at 09:03:08AM -0800, John Ousterhout wrote:
> > > A follow-up question on this, if I may. Is it OK to vmap a large
> > > region of user address space (say, 64 MB) and leave this mapped for an
> > > extended period of time (say, the life of the application), or would
> > > this have undesirable consequences? In other words, if I do this,
> > > would I need to monitor how actively the memory is being used and
> > > release the vmap for space that is inactive?
> > >
> > > -John-
> > >
> > >
> >
> > I am not an expert in this field, so the following is just my personal
> > opinion.
> >
> > When users call setsockopt(HOMA_RCV), they should be aware that this
> > memory will be occupied by the kernel no matter how large this
> > memory is, until the user explicitly notifies the kernel to release this memory
> > (HOMA can only do this through close ?).
> >
> > Therefore, my understanding is that the kernel does not
> > need to be responsible for the lifecycle of this memory. For example, if
> > the user space forgets that this registered memory has already been
> > freed, then a write from the kernel of cause could corrupt the user-space data,
> > but the kernel has no need to responsible for that.
> >
> > If you believe that this could waste memory, perhaps you should provide
> > a sparse data structure instead of a fixed memory interface.
> >
> > D. Wythe
> >
> > > On Mon, Dec 9, 2024 at 8:53 AM John Ousterhout <ouster@cs.stanford.edu> wrote:
> > > >
> > > > Thanks for the additional information; I'll put this on my list of
> > > > things to consider for performance optimization.
> > > >
> > > > -John-
> > > >
> > > >
> > > > On Sun, Dec 8, 2024 at 10:56 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 12/6/24 3:49 AM, John Ousterhout wrote:
> > > > > > On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > > > > >>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> > > > > >>> + unsigned int optlen)
> > > > > >>> +{
> > > > > >>> + struct homa_sock *hsk = homa_sk(sk);
> > > > > >>> + struct homa_set_buf_args args;
> > > > > >>> + int ret;
> > > > > >>> +
> > > > > >>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> > > > > >>> + optlen != sizeof(struct homa_set_buf_args))
> > > > > >>> + return -EINVAL;
> > > > > >>
> > > > > >> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> > > > > >> implemented in getsockopt.
> > > > > >
> > > > > > I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> > > > > > to struct homa_rcvbuf_args). I also implemented getsockopt for
> > > > > > HOMA_RCVBUF.
> > > > > >
> > > > > >>> +
> > > > > >>> + if (copy_from_sockptr(&args, optval, optlen))
> > > > > >>> + return -EFAULT;
> > > > > >>> +
> > > > > >>> + /* Do a trivial test to make sure we can at least write the first
> > > > > >>> + * page of the region.
> > > > > >>> + */
> > > > > >>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> > > > > >>> + return -EFAULT;
> > > > > >>
> > > > > >> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> > > > > >> io_pin_pbuf_ring()
> > > > > >
> > > > > > I'm not sure what you mean here. Are you suggesting that I look at the
> > > > > > code of io_pin_pbuf_ring to make sure I've done everything needed to
> > > > > > share buffers? I don't believe that Homa needs to do anything special
> > > > > > (e.g. it doesn't need to pin the user's buffers); it just saves the
> > > > > > address and makes copy_to_user calls later when needed (and these
> > > > > > calls are all done at syscall level in the context of the
> > > > > > application). Is there something I'm missing?
> > > > > >
> > > > >
> > > > > I just thought that since the received buffer is shared between kernel and user-space, if using
> > > > > vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
> > > > > from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
> > > > > relevant data to prove it..
> > > > >
> > > > > So I'm not going to insist on it, it ups to you.
> > > > >
> > > > > D. Wythe
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: John Ousterhout @ 2024-12-10 5:50 UTC (permalink / raw)
To: D. Wythe; +Cc: netdev, linux-api
In-Reply-To: <20241210051454.GA83318@j66a10360.sqa.eu95>
Thanks for the additional information. My concern was whether the
available kernel virtual address space for vmapping is a scarce
resource, in which case it might not be good for Homa to lock up large
amounts of it for long periods of time. I'm not as worried about
physical memory usage (that will happen regardless of whether the
buffers get vmapped into the kernel).
-John-
On Mon, Dec 9, 2024 at 9:14 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
>
> On Mon, Dec 09, 2024 at 09:03:08AM -0800, John Ousterhout wrote:
> > A follow-up question on this, if I may. Is it OK to vmap a large
> > region of user address space (say, 64 MB) and leave this mapped for an
> > extended period of time (say, the life of the application), or would
> > this have undesirable consequences? In other words, if I do this,
> > would I need to monitor how actively the memory is being used and
> > release the vmap for space that is inactive?
> >
> > -John-
> >
> >
>
> I am not an expert in this field, so the following is just my personal
> opinion.
>
> When users call setsockopt(HOMA_RCV), they should be aware that this
> memory will be occupied by the kernel no matter how large this
> memory is, until the user explicitly notifies the kernel to release this memory
> (HOMA can only do this through close ?).
>
> Therefore, my understanding is that the kernel does not
> need to be responsible for the lifecycle of this memory. For example, if
> the user space forgets that this registered memory has already been
> freed, then a write from the kernel of cause could corrupt the user-space data,
> but the kernel has no need to responsible for that.
>
> If you believe that this could waste memory, perhaps you should provide
> a sparse data structure instead of a fixed memory interface.
>
> D. Wythe
>
> > On Mon, Dec 9, 2024 at 8:53 AM John Ousterhout <ouster@cs.stanford.edu> wrote:
> > >
> > > Thanks for the additional information; I'll put this on my list of
> > > things to consider for performance optimization.
> > >
> > > -John-
> > >
> > >
> > > On Sun, Dec 8, 2024 at 10:56 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > > >
> > > >
> > > >
> > > > On 12/6/24 3:49 AM, John Ousterhout wrote:
> > > > > On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > > > >>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> > > > >>> + unsigned int optlen)
> > > > >>> +{
> > > > >>> + struct homa_sock *hsk = homa_sk(sk);
> > > > >>> + struct homa_set_buf_args args;
> > > > >>> + int ret;
> > > > >>> +
> > > > >>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> > > > >>> + optlen != sizeof(struct homa_set_buf_args))
> > > > >>> + return -EINVAL;
> > > > >>
> > > > >> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> > > > >> implemented in getsockopt.
> > > > >
> > > > > I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> > > > > to struct homa_rcvbuf_args). I also implemented getsockopt for
> > > > > HOMA_RCVBUF.
> > > > >
> > > > >>> +
> > > > >>> + if (copy_from_sockptr(&args, optval, optlen))
> > > > >>> + return -EFAULT;
> > > > >>> +
> > > > >>> + /* Do a trivial test to make sure we can at least write the first
> > > > >>> + * page of the region.
> > > > >>> + */
> > > > >>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> > > > >>> + return -EFAULT;
> > > > >>
> > > > >> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> > > > >> io_pin_pbuf_ring()
> > > > >
> > > > > I'm not sure what you mean here. Are you suggesting that I look at the
> > > > > code of io_pin_pbuf_ring to make sure I've done everything needed to
> > > > > share buffers? I don't believe that Homa needs to do anything special
> > > > > (e.g. it doesn't need to pin the user's buffers); it just saves the
> > > > > address and makes copy_to_user calls later when needed (and these
> > > > > calls are all done at syscall level in the context of the
> > > > > application). Is there something I'm missing?
> > > > >
> > > >
> > > > I just thought that since the received buffer is shared between kernel and user-space, if using
> > > > vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
> > > > from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
> > > > relevant data to prove it..
> > > >
> > > > So I'm not going to insist on it, it ups to you.
> > > >
> > > > D. Wythe
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: D. Wythe @ 2024-12-10 5:14 UTC (permalink / raw)
To: John Ousterhout; +Cc: D. Wythe, netdev, linux-api
In-Reply-To: <CAGXJAmzzmNrp-7OANK1yQX+xJcGAYjTWDFuyhdD5pvjN28CtXQ@mail.gmail.com>
On Mon, Dec 09, 2024 at 09:03:08AM -0800, John Ousterhout wrote:
> A follow-up question on this, if I may. Is it OK to vmap a large
> region of user address space (say, 64 MB) and leave this mapped for an
> extended period of time (say, the life of the application), or would
> this have undesirable consequences? In other words, if I do this,
> would I need to monitor how actively the memory is being used and
> release the vmap for space that is inactive?
>
> -John-
>
>
I am not an expert in this field, so the following is just my personal
opinion.
When users call setsockopt(HOMA_RCV), they should be aware that this
memory will be occupied by the kernel no matter how large this
memory is, until the user explicitly notifies the kernel to release this memory
(HOMA can only do this through close ?).
Therefore, my understanding is that the kernel does not
need to be responsible for the lifecycle of this memory. For example, if
the user space forgets that this registered memory has already been
freed, then a write from the kernel of cause could corrupt the user-space data,
but the kernel has no need to responsible for that.
If you believe that this could waste memory, perhaps you should provide
a sparse data structure instead of a fixed memory interface.
D. Wythe
> On Mon, Dec 9, 2024 at 8:53 AM John Ousterhout <ouster@cs.stanford.edu> wrote:
> >
> > Thanks for the additional information; I'll put this on my list of
> > things to consider for performance optimization.
> >
> > -John-
> >
> >
> > On Sun, Dec 8, 2024 at 10:56 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > >
> > >
> > >
> > > On 12/6/24 3:49 AM, John Ousterhout wrote:
> > > > On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > > >>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> > > >>> + unsigned int optlen)
> > > >>> +{
> > > >>> + struct homa_sock *hsk = homa_sk(sk);
> > > >>> + struct homa_set_buf_args args;
> > > >>> + int ret;
> > > >>> +
> > > >>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> > > >>> + optlen != sizeof(struct homa_set_buf_args))
> > > >>> + return -EINVAL;
> > > >>
> > > >> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> > > >> implemented in getsockopt.
> > > >
> > > > I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> > > > to struct homa_rcvbuf_args). I also implemented getsockopt for
> > > > HOMA_RCVBUF.
> > > >
> > > >>> +
> > > >>> + if (copy_from_sockptr(&args, optval, optlen))
> > > >>> + return -EFAULT;
> > > >>> +
> > > >>> + /* Do a trivial test to make sure we can at least write the first
> > > >>> + * page of the region.
> > > >>> + */
> > > >>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> > > >>> + return -EFAULT;
> > > >>
> > > >> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> > > >> io_pin_pbuf_ring()
> > > >
> > > > I'm not sure what you mean here. Are you suggesting that I look at the
> > > > code of io_pin_pbuf_ring to make sure I've done everything needed to
> > > > share buffers? I don't believe that Homa needs to do anything special
> > > > (e.g. it doesn't need to pin the user's buffers); it just saves the
> > > > address and makes copy_to_user calls later when needed (and these
> > > > calls are all done at syscall level in the context of the
> > > > application). Is there something I'm missing?
> > > >
> > >
> > > I just thought that since the received buffer is shared between kernel and user-space, if using
> > > vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
> > > from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
> > > relevant data to prove it..
> > >
> > > So I'm not going to insist on it, it ups to you.
> > >
> > > D. Wythe
^ permalink raw reply
* Re: [PATCH v6 00/28] NT synchronization primitive driver
From: Elizabeth Figura @ 2024-12-09 22:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jonathan Corbet, shuah, Arnd Bergmann
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng
In-Reply-To: <a6ec954d-7794-483d-a1f3-d58d33c5fc7a@app.fastmail.com>
On Monday, 9 December 2024 14:24:36 CST Arnd Bergmann wrote:
> On Mon, Dec 9, 2024, at 19:58, Elizabeth Figura wrote:
> > == Previous versions ==
> >
> > No changes were made from v5 other than rebasing on top of the 6.13-rc1
> > char-misc-next tree.
> >
> > I would like to repeat a question from the last round of review, though. Two
> > changes were suggested related to API design, which I did not make because the
> > APIs in question were already released in upstream Linux. However, the driver is
> > also completely nonfunctional and hidden behind BROKEN, so would this be
> > acceptable anyway? The changes in question are:
>
> If it was impossible to use the driver, there is no regression.
> I feel the entire point of marking it as broken was to be able
> to add that type of change.
Makes sense. [I figured that the BROKEN was just there to prevent anyone from trying to use a half-finished driver, and the point of committing a half-finished driver was just to reduce the number of patches that needed to be resent.]
I'll make these changes and resend.
> > * rename NTSYNC_IOC_SEM_POST to NTSYNC_IOC_SEM_RELEASE (matching the NT
> > terminology instead of POSIX),
>
> No objections my me on either name.
>
> > * change object creation ioctls to return the fds directly in the return value
> > instead of through the args struct. I would also still appreciate a
> > clarification on the advice in [1], which is why I didn't do this in the first
> > place.
> >
> > [1] https://docs.kernel.org/driver-api/ioctl.html#return-code
>
> The git log tells me that I have written that, but I don't remember
> why I put that in, maybe someone else suggested it.
>
> My feeling right now is that returning a file descriptor number
> as a small positive integer from the ioctl() return code makes
> sense. On the other hand, returning pointers, negative signed
> integers or large (> 32bit) 'unsigned long' values can cause
> a number of issues, so I would avoid all those the same way we
> discourage passing those integers as a literal 'arg' into ioctl()
> instead of going through a pointer.
Ah, that makes sense to me, thanks.
^ permalink raw reply
* Re: [PATCH v6 00/28] NT synchronization primitive driver
From: Arnd Bergmann @ 2024-12-09 20:24 UTC (permalink / raw)
To: Elizabeth Figura, Greg Kroah-Hartman, Jonathan Corbet, shuah
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
On Mon, Dec 9, 2024, at 19:58, Elizabeth Figura wrote:
> == Previous versions ==
>
> No changes were made from v5 other than rebasing on top of the 6.13-rc1
> char-misc-next tree.
>
> I would like to repeat a question from the last round of review, though. Two
> changes were suggested related to API design, which I did not make because the
> APIs in question were already released in upstream Linux. However, the driver is
> also completely nonfunctional and hidden behind BROKEN, so would this be
> acceptable anyway? The changes in question are:
If it was impossible to use the driver, there is no regression.
I feel the entire point of marking it as broken was to be able
to add that type of change.
> * rename NTSYNC_IOC_SEM_POST to NTSYNC_IOC_SEM_RELEASE (matching the NT
> terminology instead of POSIX),
No objections my me on either name.
> * change object creation ioctls to return the fds directly in the return value
> instead of through the args struct. I would also still appreciate a
> clarification on the advice in [1], which is why I didn't do this in the first
> place.
>
> [1] https://docs.kernel.org/driver-api/ioctl.html#return-code
The git log tells me that I have written that, but I don't remember
why I put that in, maybe someone else suggested it.
My feeling right now is that returning a file descriptor number
as a small positive integer from the ioctl() return code makes
sense. On the other hand, returning pointers, negative signed
integers or large (> 32bit) 'unsigned long' values can cause
a number of issues, so I would avoid all those the same way we
discourage passing those integers as a literal 'arg' into ioctl()
instead of going through a pointer.
So either way, this looks good to me. I also looked through the
series again to double-check that you avoid the usual common
problems we list in Documentation/driver-api/ioctl.rst, and
I found this is all fine.
So with or without the two changes you listed:
Acked-by: Arnd Bergmann <arnd@arndb.de>
Arnd
^ permalink raw reply
* [PATCH v6 16/28] selftests: ntsync: Add some tests for NTSYNC_IOC_WAIT_ANY.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Test basic synchronous functionality of NTSYNC_IOC_WAIT_ANY, when objects are
considered signaled or not signaled, and how they are affected by a successful
wait.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 119 ++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 7cd0f40594fd..40ad8cbd3138 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -342,4 +342,123 @@ TEST(mutex_state)
close(fd);
}
+TEST(test_wait_any)
+{
+ int objs[NTSYNC_MAX_WAIT_COUNT + 1], fd, ret;
+ struct ntsync_mutex_args mutex_args = {0};
+ struct ntsync_sem_args sem_args = {0};
+ __u32 owner, index, count, i;
+ struct timespec timeout;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ sem_args.count = 2;
+ sem_args.max = 3;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, sem_args.sem);
+
+ mutex_args.owner = 0;
+ mutex_args.count = 0;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+
+ objs[0] = sem_args.sem;
+ objs[1] = mutex_args.mutex;
+
+ ret = wait_any(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 1, 3);
+ check_mutex_state(mutex_args.mutex, 0, 0);
+
+ ret = wait_any(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_mutex_state(mutex_args.mutex, 0, 0);
+
+ ret = wait_any(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, index);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_mutex_state(mutex_args.mutex, 1, 123);
+
+ count = 1;
+ ret = post_sem(sem_args.sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+
+ ret = wait_any(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_mutex_state(mutex_args.mutex, 1, 123);
+
+ ret = wait_any(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, index);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_mutex_state(mutex_args.mutex, 2, 123);
+
+ ret = wait_any(fd, 2, objs, 456, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ owner = 123;
+ ret = ioctl(mutex_args.mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_any(fd, 2, objs, 456, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(1, index);
+
+ ret = wait_any(fd, 2, objs, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, index);
+
+ /* test waiting on the same object twice */
+ count = 2;
+ ret = post_sem(sem_args.sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+
+ objs[0] = objs[1] = sem_args.sem;
+ ret = wait_any(fd, 2, objs, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 1, 3);
+
+ ret = wait_any(fd, 0, NULL, 456, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ for (i = 0; i < NTSYNC_MAX_WAIT_COUNT + 1; ++i)
+ objs[i] = sem_args.sem;
+
+ ret = wait_any(fd, NTSYNC_MAX_WAIT_COUNT, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ ret = wait_any(fd, NTSYNC_MAX_WAIT_COUNT + 1, objs, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ ret = wait_any(fd, -1, objs, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ close(sem_args.sem);
+ close(mutex_args.mutex);
+
+ close(fd);
+}
+
TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
* [PATCH v6 07/28] ntsync: Introduce NTSYNC_IOC_EVENT_SET.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtSetEvent().
This sets the event to the signaled state, and returns its previous state.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 27 +++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index ef8949ee2d4b..e46dd795370a 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -534,6 +534,31 @@ static int ntsync_mutex_kill(struct ntsync_obj *mutex, void __user *argp)
return ret;
}
+static int ntsync_event_set(struct ntsync_obj *event, void __user *argp)
+{
+ struct ntsync_device *dev = event->dev;
+ __u32 prev_state;
+ bool all;
+
+ if (event->type != NTSYNC_TYPE_EVENT)
+ return -EINVAL;
+
+ all = ntsync_lock_obj(dev, event);
+
+ prev_state = event->u.event.signaled;
+ event->u.event.signaled = true;
+ if (all)
+ try_wake_all_obj(dev, event);
+ try_wake_any_event(event);
+
+ ntsync_unlock_obj(dev, event, all);
+
+ if (put_user(prev_state, (__u32 __user *)argp))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -557,6 +582,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
return ntsync_mutex_unlock(obj, argp);
case NTSYNC_IOC_MUTEX_KILL:
return ntsync_mutex_kill(obj, argp);
+ case NTSYNC_IOC_EVENT_SET:
+ return ntsync_event_set(obj, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 4c0c4271c7de..36d903521bbe 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -51,5 +51,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
#define NTSYNC_IOC_MUTEX_KILL _IOW ('N', 0x86, __u32)
+#define NTSYNC_IOC_EVENT_SET _IOR ('N', 0x88, __u32)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 00/28] NT synchronization primitive driver
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
This patch series implements a new char misc driver, /dev/ntsync, which is used
to implement Windows NT synchronization primitives.
NT synchronization primitives are unique in that the wait functions both are
vectored, operate on multiple types of object with different behaviour (mutex,
semaphore, event), and affect the state of the objects they wait on. This model
is not compatible with existing kernel synchronization objects or interfaces,
and therefore the ntsync driver implements its own wait queues and locking.
This patch series is rebased against the "char-misc-next" branch of
gregkh/char-misc.git.
== Background ==
The Wine project emulates the Windows API in user space. One particular part of
that API, namely the NT synchronization primitives, have historically been
implemented via RPC to a dedicated "kernel" process. However, more recent
applications use these APIs more strenuously, and the overhead of RPC has become
a bottleneck.
The NT synchronization APIs are too complex to implement on top of existing
primitives without sacrificing correctness. Certain operations, such as
NtPulseEvent() or the "wait-for-all" mode of NtWaitForMultipleObjects(), require
direct control over the underlying wait queue, and implementing a wait queue
sufficiently robust for Wine in user space is not possible. This proposed
driver, therefore, implements the problematic interfaces directly in the Linux
kernel.
This driver was presented at Linux Plumbers Conference 2023. For those further
interested in the history of synchronization in Wine and past attempts to solve
this problem in user space, a recording of the presentation can be viewed here:
https://www.youtube.com/watch?v=NjU4nyWyhU8
== Performance ==
The performance measurements described below are copied from earlier versions of
the patch set. While some of the code has changed, I do not currently anticipate
that it has changed drastically enough to affect those measurements.
The gain in performance varies wildly depending on the application in question
and the user's hardware. For some games NT synchronization is not a bottleneck
and no change can be observed, but for others frame rate improvements of 50 to
150 percent are not atypical. The following table lists frame rate measurements
from a variety of games on a variety of hardware, taken by users Dmitry
Skvortsov, FuzzyQuils, OnMars, and myself:
Game Upstream ntsync improvement
===========================================================================
Anger Foot 69 99 43%
Call of Juarez 99.8 224.1 125%
Dirt 3 110.6 860.7 678%
Forza Horizon 5 108 160 48%
Lara Croft: Temple of Osiris 141 326 131%
Metro 2033 164.4 199.2 21%
Resident Evil 2 26 77 196%
The Crew 26 51 96%
Tiny Tina's Wonderlands 130 360 177%
Total War Saga: Troy 109 146 34%
===========================================================================
== Patches ==
The intended semantics of the patches are broadly intended to match those of the
corresponding Windows functions. For those not already familiar with the Windows
functions (or their undocumented behaviour), patch 27/28 provides a detailed
specification, and individual patches also include a brief description of the
API they are implementing.
The patches making use of this driver in Wine can be retrieved or browsed here:
https://repo.or.cz/wine/zf.git/shortlog/refs/heads/ntsync5
== Previous versions ==
No changes were made from v5 other than rebasing on top of the 6.13-rc1
char-misc-next tree.
I would like to repeat a question from the last round of review, though. Two
changes were suggested related to API design, which I did not make because the
APIs in question were already released in upstream Linux. However, the driver is
also completely nonfunctional and hidden behind BROKEN, so would this be
acceptable anyway? The changes in question are:
* rename NTSYNC_IOC_SEM_POST to NTSYNC_IOC_SEM_RELEASE (matching the NT
terminology instead of POSIX),
* change object creation ioctls to return the fds directly in the return value
instead of through the args struct. I would also still appreciate a
clarification on the advice in [1], which is why I didn't do this in the first
place.
[1] https://docs.kernel.org/driver-api/ioctl.html#return-code
* Link to v5: https://lore.kernel.org/lkml/20240519202454.1192826-1-zfigura@codeweavers.com/
* Link to v4: https://lore.kernel.org/lkml/20240416010837.333694-1-zfigura@codeweavers.com/
* Link to v3: https://lore.kernel.org/lkml/20240329000621.148791-1-zfigura@codeweavers.com/
* Link to v2: https://lore.kernel.org/lkml/20240219223833.95710-1-zfigura@codeweavers.com/
* Link to v1: https://lore.kernel.org/lkml/20240214233645.9273-1-zfigura@codeweavers.com/
* Link to RFC v2: https://lore.kernel.org/lkml/20240131021356.10322-1-zfigura@codeweavers.com/
* Link to RFC v1: https://lore.kernel.org/lkml/20240124004028.16826-1-zfigura@codeweavers.com/
Elizabeth Figura (28):
ntsync: Introduce NTSYNC_IOC_WAIT_ANY.
ntsync: Introduce NTSYNC_IOC_WAIT_ALL.
ntsync: Introduce NTSYNC_IOC_CREATE_MUTEX.
ntsync: Introduce NTSYNC_IOC_MUTEX_UNLOCK.
ntsync: Introduce NTSYNC_IOC_MUTEX_KILL.
ntsync: Introduce NTSYNC_IOC_CREATE_EVENT.
ntsync: Introduce NTSYNC_IOC_EVENT_SET.
ntsync: Introduce NTSYNC_IOC_EVENT_RESET.
ntsync: Introduce NTSYNC_IOC_EVENT_PULSE.
ntsync: Introduce NTSYNC_IOC_SEM_READ.
ntsync: Introduce NTSYNC_IOC_MUTEX_READ.
ntsync: Introduce NTSYNC_IOC_EVENT_READ.
ntsync: Introduce alertable waits.
selftests: ntsync: Add some tests for semaphore state.
selftests: ntsync: Add some tests for mutex state.
selftests: ntsync: Add some tests for NTSYNC_IOC_WAIT_ANY.
selftests: ntsync: Add some tests for NTSYNC_IOC_WAIT_ALL.
selftests: ntsync: Add some tests for wakeup signaling with
WINESYNC_IOC_WAIT_ANY.
selftests: ntsync: Add some tests for wakeup signaling with
WINESYNC_IOC_WAIT_ALL.
selftests: ntsync: Add some tests for manual-reset event state.
selftests: ntsync: Add some tests for auto-reset event state.
selftests: ntsync: Add some tests for wakeup signaling with events.
selftests: ntsync: Add tests for alertable waits.
selftests: ntsync: Add some tests for wakeup signaling via alerts.
selftests: ntsync: Add a stress test for contended waits.
maintainers: Add an entry for ntsync.
docs: ntsync: Add documentation for the ntsync uAPI.
ntsync: No longer depend on BROKEN.
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/ntsync.rst | 398 +++++
MAINTAINERS | 9 +
drivers/misc/Kconfig | 1 -
drivers/misc/ntsync.c | 989 +++++++++++-
include/uapi/linux/ntsync.h | 39 +
tools/testing/selftests/Makefile | 1 +
.../selftests/drivers/ntsync/.gitignore | 1 +
.../testing/selftests/drivers/ntsync/Makefile | 7 +
tools/testing/selftests/drivers/ntsync/config | 1 +
.../testing/selftests/drivers/ntsync/ntsync.c | 1407 +++++++++++++++++
11 files changed, 2850 insertions(+), 4 deletions(-)
create mode 100644 Documentation/userspace-api/ntsync.rst
create mode 100644 tools/testing/selftests/drivers/ntsync/.gitignore
create mode 100644 tools/testing/selftests/drivers/ntsync/Makefile
create mode 100644 tools/testing/selftests/drivers/ntsync/config
create mode 100644 tools/testing/selftests/drivers/ntsync/ntsync.c
base-commit: cdd30ebb1b9f36159d66f088b61aee264e649d7a
--
2.45.2
^ permalink raw reply
* [PATCH v6 02/28] ntsync: Introduce NTSYNC_IOC_WAIT_ALL.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This is similar to NTSYNC_IOC_WAIT_ANY, but waits until all of the objects are
simultaneously signaled, and then acquires all of them as a single atomic
operation.
Because acquisition of multiple objects is atomic, some complex locking is
required. We cannot simply spin-lock multiple objects simultaneously, as that
may disable preëmption for a problematically long time.
Instead, modifying any object which may be involved in a wait-all operation takes
a device-wide sleeping mutex, "wait_all_lock", instead of the normal object
spinlock.
Because wait-for-all is a rare operation, in order to optimize wait-for-any,
this lock is only taken when necessary. "all_hint" is used to mark objects which
are involved in a wait-for-all operation, and if an object is not, only its
spinlock is taken.
The locking scheme used here was written by Peter Zijlstra.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 334 ++++++++++++++++++++++++++++++++++--
include/uapi/linux/ntsync.h | 1 +
2 files changed, 322 insertions(+), 13 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 2bfe5299f25b..0f4da571fa33 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -13,6 +13,7 @@
#include <linux/ktime.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/overflow.h>
#include <linux/sched.h>
#include <linux/sched/signal.h>
@@ -41,6 +42,7 @@ enum ntsync_type {
struct ntsync_obj {
spinlock_t lock;
+ int dev_locked;
enum ntsync_type type;
@@ -55,7 +57,30 @@ struct ntsync_obj {
} sem;
} u;
+ /*
+ * any_waiters is protected by the object lock, but all_waiters is
+ * protected by the device wait_all_lock.
+ */
struct list_head any_waiters;
+ struct list_head all_waiters;
+
+ /*
+ * Hint describing how many tasks are queued on this object in a
+ * wait-all operation.
+ *
+ * Any time we do a wake, we may need to wake "all" waiters as well as
+ * "any" waiters. In order to atomically wake "all" waiters, we must
+ * lock all of the objects, and that means grabbing the wait_all_lock
+ * below (and, due to lock ordering rules, before locking this object).
+ * However, wait-all is a rare operation, and grabbing the wait-all
+ * lock for every wake would create unnecessary contention.
+ * Therefore we first check whether all_hint is zero, and, if it is,
+ * we skip trying to wake "all" waiters.
+ *
+ * Since wait requests must originate from user-space threads, we're
+ * limited here by PID_MAX_LIMIT, so there's no risk of overflow.
+ */
+ atomic_t all_hint;
};
struct ntsync_q_entry {
@@ -75,19 +100,198 @@ struct ntsync_q {
*/
atomic_t signaled;
+ bool all;
__u32 count;
struct ntsync_q_entry entries[];
};
struct ntsync_device {
+ /*
+ * Wait-all operations must atomically grab all objects, and be totally
+ * ordered with respect to each other and wait-any operations.
+ * If one thread is trying to acquire several objects, another thread
+ * cannot touch the object at the same time.
+ *
+ * This device-wide lock is used to serialize wait-for-all
+ * operations, and operations on an object that is involved in a
+ * wait-for-all.
+ */
+ struct mutex wait_all_lock;
+
struct file *file;
};
+/*
+ * Single objects are locked using obj->lock.
+ *
+ * Multiple objects are 'locked' while holding dev->wait_all_lock.
+ * In this case however, individual objects are not locked by holding
+ * obj->lock, but by setting obj->dev_locked.
+ *
+ * This means that in order to lock a single object, the sequence is slightly
+ * more complicated than usual. Specifically it needs to check obj->dev_locked
+ * after acquiring obj->lock, if set, it needs to drop the lock and acquire
+ * dev->wait_all_lock in order to serialize against the multi-object operation.
+ */
+
+static void dev_lock_obj(struct ntsync_device *dev, struct ntsync_obj *obj)
+{
+ lockdep_assert_held(&dev->wait_all_lock);
+ lockdep_assert(obj->dev == dev);
+ spin_lock(&obj->lock);
+ /*
+ * By setting obj->dev_locked inside obj->lock, it is ensured that
+ * anyone holding obj->lock must see the value.
+ */
+ obj->dev_locked = 1;
+ spin_unlock(&obj->lock);
+}
+
+static void dev_unlock_obj(struct ntsync_device *dev, struct ntsync_obj *obj)
+{
+ lockdep_assert_held(&dev->wait_all_lock);
+ lockdep_assert(obj->dev == dev);
+ spin_lock(&obj->lock);
+ obj->dev_locked = 0;
+ spin_unlock(&obj->lock);
+}
+
+static void obj_lock(struct ntsync_obj *obj)
+{
+ struct ntsync_device *dev = obj->dev;
+
+ for (;;) {
+ spin_lock(&obj->lock);
+ if (likely(!obj->dev_locked))
+ break;
+
+ spin_unlock(&obj->lock);
+ mutex_lock(&dev->wait_all_lock);
+ spin_lock(&obj->lock);
+ /*
+ * obj->dev_locked should be set and released under the same
+ * wait_all_lock section, since we now own this lock, it should
+ * be clear.
+ */
+ lockdep_assert(!obj->dev_locked);
+ spin_unlock(&obj->lock);
+ mutex_unlock(&dev->wait_all_lock);
+ }
+}
+
+static void obj_unlock(struct ntsync_obj *obj)
+{
+ spin_unlock(&obj->lock);
+}
+
+static bool ntsync_lock_obj(struct ntsync_device *dev, struct ntsync_obj *obj)
+{
+ bool all;
+
+ obj_lock(obj);
+ all = atomic_read(&obj->all_hint);
+ if (unlikely(all)) {
+ obj_unlock(obj);
+ mutex_lock(&dev->wait_all_lock);
+ dev_lock_obj(dev, obj);
+ }
+
+ return all;
+}
+
+static void ntsync_unlock_obj(struct ntsync_device *dev, struct ntsync_obj *obj, bool all)
+{
+ if (all) {
+ dev_unlock_obj(dev, obj);
+ mutex_unlock(&dev->wait_all_lock);
+ } else {
+ obj_unlock(obj);
+ }
+}
+
+#define ntsync_assert_held(obj) \
+ lockdep_assert((lockdep_is_held(&(obj)->lock) != LOCK_STATE_NOT_HELD) || \
+ ((lockdep_is_held(&(obj)->dev->wait_all_lock) != LOCK_STATE_NOT_HELD) && \
+ (obj)->dev_locked))
+
+static bool is_signaled(struct ntsync_obj *obj)
+{
+ ntsync_assert_held(obj);
+
+ switch (obj->type) {
+ case NTSYNC_TYPE_SEM:
+ return !!obj->u.sem.count;
+ }
+
+ WARN(1, "bad object type %#x\n", obj->type);
+ return false;
+}
+
+/*
+ * "locked_obj" is an optional pointer to an object which is already locked and
+ * should not be locked again. This is necessary so that changing an object's
+ * state and waking it can be a single atomic operation.
+ */
+static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
+ struct ntsync_obj *locked_obj)
+{
+ __u32 count = q->count;
+ bool can_wake = true;
+ int signaled = -1;
+ __u32 i;
+
+ lockdep_assert_held(&dev->wait_all_lock);
+ if (locked_obj)
+ lockdep_assert(locked_obj->dev_locked);
+
+ for (i = 0; i < count; i++) {
+ if (q->entries[i].obj != locked_obj)
+ dev_lock_obj(dev, q->entries[i].obj);
+ }
+
+ for (i = 0; i < count; i++) {
+ if (!is_signaled(q->entries[i].obj)) {
+ can_wake = false;
+ break;
+ }
+ }
+
+ if (can_wake && atomic_try_cmpxchg(&q->signaled, &signaled, 0)) {
+ for (i = 0; i < count; i++) {
+ struct ntsync_obj *obj = q->entries[i].obj;
+
+ switch (obj->type) {
+ case NTSYNC_TYPE_SEM:
+ obj->u.sem.count--;
+ break;
+ }
+ }
+ wake_up_process(q->task);
+ }
+
+ for (i = 0; i < count; i++) {
+ if (q->entries[i].obj != locked_obj)
+ dev_unlock_obj(dev, q->entries[i].obj);
+ }
+}
+
+static void try_wake_all_obj(struct ntsync_device *dev, struct ntsync_obj *obj)
+{
+ struct ntsync_q_entry *entry;
+
+ lockdep_assert_held(&dev->wait_all_lock);
+ lockdep_assert(obj->dev_locked);
+
+ list_for_each_entry(entry, &obj->all_waiters, node)
+ try_wake_all(dev, entry->q, obj);
+}
+
static void try_wake_any_sem(struct ntsync_obj *sem)
{
struct ntsync_q_entry *entry;
- lockdep_assert_held(&sem->lock);
+ ntsync_assert_held(sem);
+ lockdep_assert(sem->type == NTSYNC_TYPE_SEM);
list_for_each_entry(entry, &sem->any_waiters, node) {
struct ntsync_q *q = entry->q;
@@ -111,7 +315,7 @@ static int post_sem_state(struct ntsync_obj *sem, __u32 count)
{
__u32 sum;
- lockdep_assert_held(&sem->lock);
+ ntsync_assert_held(sem);
if (check_add_overflow(sem->u.sem.count, count, &sum) ||
sum > sem->u.sem.max)
@@ -123,9 +327,11 @@ static int post_sem_state(struct ntsync_obj *sem, __u32 count)
static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
{
+ struct ntsync_device *dev = sem->dev;
__u32 __user *user_args = argp;
__u32 prev_count;
__u32 args;
+ bool all;
int ret;
if (copy_from_user(&args, argp, sizeof(args)))
@@ -134,14 +340,17 @@ static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
if (sem->type != NTSYNC_TYPE_SEM)
return -EINVAL;
- spin_lock(&sem->lock);
+ all = ntsync_lock_obj(dev, sem);
prev_count = sem->u.sem.count;
ret = post_sem_state(sem, args);
- if (!ret)
+ if (!ret) {
+ if (all)
+ try_wake_all_obj(dev, sem);
try_wake_any_sem(sem);
+ }
- spin_unlock(&sem->lock);
+ ntsync_unlock_obj(dev, sem, all);
if (!ret && put_user(prev_count, user_args))
ret = -EFAULT;
@@ -193,6 +402,8 @@ static struct ntsync_obj *ntsync_alloc_obj(struct ntsync_device *dev,
get_file(dev->file);
spin_lock_init(&obj->lock);
INIT_LIST_HEAD(&obj->any_waiters);
+ INIT_LIST_HEAD(&obj->all_waiters);
+ atomic_set(&obj->all_hint, 0);
return obj;
}
@@ -304,7 +515,7 @@ static int ntsync_schedule(const struct ntsync_q *q, const struct ntsync_wait_ar
* Allocate and initialize the ntsync_q structure, but do not queue us yet.
*/
static int setup_wait(struct ntsync_device *dev,
- const struct ntsync_wait_args *args,
+ const struct ntsync_wait_args *args, bool all,
struct ntsync_q **ret_q)
{
const __u32 count = args->count;
@@ -327,6 +538,7 @@ static int setup_wait(struct ntsync_device *dev,
return -ENOMEM;
q->task = current;
atomic_set(&q->signaled, -1);
+ q->all = all;
q->count = count;
for (i = 0; i < count; i++) {
@@ -336,6 +548,16 @@ static int setup_wait(struct ntsync_device *dev,
if (!obj)
goto err;
+ if (all) {
+ /* Check that the objects are all distinct. */
+ for (j = 0; j < i; j++) {
+ if (obj == q->entries[j].obj) {
+ put_obj(obj);
+ goto err;
+ }
+ }
+ }
+
entry->obj = obj;
entry->q = q;
entry->index = i;
@@ -365,13 +587,14 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
struct ntsync_wait_args args;
struct ntsync_q *q;
int signaled;
+ bool all;
__u32 i;
int ret;
if (copy_from_user(&args, argp, sizeof(args)))
return -EFAULT;
- ret = setup_wait(dev, &args, &q);
+ ret = setup_wait(dev, &args, false, &q);
if (ret < 0)
return ret;
@@ -381,9 +604,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
struct ntsync_q_entry *entry = &q->entries[i];
struct ntsync_obj *obj = entry->obj;
- spin_lock(&obj->lock);
+ all = ntsync_lock_obj(dev, obj);
list_add_tail(&entry->node, &obj->any_waiters);
- spin_unlock(&obj->lock);
+ ntsync_unlock_obj(dev, obj, all);
}
/* check if we are already signaled */
@@ -394,9 +617,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
if (atomic_read(&q->signaled) != -1)
break;
- spin_lock(&obj->lock);
+ all = ntsync_lock_obj(dev, obj);
try_wake_any_obj(obj);
- spin_unlock(&obj->lock);
+ ntsync_unlock_obj(dev, obj, all);
}
/* sleep */
@@ -409,9 +632,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
struct ntsync_q_entry *entry = &q->entries[i];
struct ntsync_obj *obj = entry->obj;
- spin_lock(&obj->lock);
+ all = ntsync_lock_obj(dev, obj);
list_del(&entry->node);
- spin_unlock(&obj->lock);
+ ntsync_unlock_obj(dev, obj, all);
put_obj(obj);
}
@@ -433,6 +656,87 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
return ret;
}
+static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_wait_args args;
+ struct ntsync_q *q;
+ int signaled;
+ __u32 i;
+ int ret;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ ret = setup_wait(dev, &args, true, &q);
+ if (ret < 0)
+ return ret;
+
+ /* queue ourselves */
+
+ mutex_lock(&dev->wait_all_lock);
+
+ for (i = 0; i < args.count; i++) {
+ struct ntsync_q_entry *entry = &q->entries[i];
+ struct ntsync_obj *obj = entry->obj;
+
+ atomic_inc(&obj->all_hint);
+
+ /*
+ * obj->all_waiters is protected by dev->wait_all_lock rather
+ * than obj->lock, so there is no need to acquire obj->lock
+ * here.
+ */
+ list_add_tail(&entry->node, &obj->all_waiters);
+ }
+
+ /* check if we are already signaled */
+
+ try_wake_all(dev, q, NULL);
+
+ mutex_unlock(&dev->wait_all_lock);
+
+ /* sleep */
+
+ ret = ntsync_schedule(q, &args);
+
+ /* and finally, unqueue */
+
+ mutex_lock(&dev->wait_all_lock);
+
+ for (i = 0; i < args.count; i++) {
+ struct ntsync_q_entry *entry = &q->entries[i];
+ struct ntsync_obj *obj = entry->obj;
+
+ /*
+ * obj->all_waiters is protected by dev->wait_all_lock rather
+ * than obj->lock, so there is no need to acquire it here.
+ */
+ list_del(&entry->node);
+
+ atomic_dec(&obj->all_hint);
+
+ put_obj(obj);
+ }
+
+ mutex_unlock(&dev->wait_all_lock);
+
+ signaled = atomic_read(&q->signaled);
+ if (signaled != -1) {
+ struct ntsync_wait_args __user *user_args = argp;
+
+ /* even if we caught a signal, we need to communicate success */
+ ret = 0;
+
+ if (put_user(signaled, &user_args->index))
+ ret = -EFAULT;
+ } else if (!ret) {
+ ret = -ETIMEDOUT;
+ }
+
+ kfree(q);
+ return ret;
+}
+
static int ntsync_char_open(struct inode *inode, struct file *file)
{
struct ntsync_device *dev;
@@ -441,6 +745,8 @@ static int ntsync_char_open(struct inode *inode, struct file *file)
if (!dev)
return -ENOMEM;
+ mutex_init(&dev->wait_all_lock);
+
file->private_data = dev;
dev->file = file;
return nonseekable_open(inode, file);
@@ -464,6 +770,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case NTSYNC_IOC_CREATE_SEM:
return ntsync_create_sem(dev, argp);
+ case NTSYNC_IOC_WAIT_ALL:
+ return ntsync_wait_all(dev, argp);
case NTSYNC_IOC_WAIT_ANY:
return ntsync_wait_any(dev, argp);
default:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index edc12c7a10dc..addf187b1573 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -31,6 +31,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_CREATE_SEM _IOWR('N', 0x80, struct ntsync_sem_args)
#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
+#define NTSYNC_IOC_WAIT_ALL _IOWR('N', 0x83, struct ntsync_wait_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v6 01/28] ntsync: Introduce NTSYNC_IOC_WAIT_ANY.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to part of the functionality of the NT syscall
NtWaitForMultipleObjects(). Specifically, it implements the behaviour where
the third argument (wait_any) is TRUE, and it does not handle alertable waits.
Those features have been split out into separate patches to ease review.
This patch therefore implements the wait/wake infrastructure which comprises the
core of ntsync's functionality.
NTSYNC_IOC_WAIT_ANY is a vectored wait function similar to poll(). Unlike
poll(), it "consumes" objects when they are signaled. For semaphores, this means
decreasing one from the internal counter. At most one object can be consumed by
this function.
This wait/wake model is fundamentally different from that used anywhere else in
the kernel, and for that reason ntsync does not use any existing infrastructure,
such as futexes, kernel mutexes or semaphores, or wait_event().
Up to 64 objects can be waited on at once. As soon as one is signaled, the
object with the lowest index is consumed, and that index is returned via the
"index" field.
A timeout is supported. The timeout is passed as a u64 nanosecond value, which
represents absolute time measured against either the MONOTONIC or REALTIME clock
(controlled by the flags argument). If U64_MAX is passed, the ioctl waits
indefinitely.
This ioctl validates that all objects belong to the relevant device. This is not
necessary for any technical reason related to NTSYNC_IOC_WAIT_ANY, but will be
necessary for NTSYNC_IOC_WAIT_ALL introduced in the following patch.
Some padding fields are added for alignment and for fields which will be added
in future patches (split out to ease review).
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 245 ++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 14 +++
2 files changed, 259 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 4954553b7baa..2bfe5299f25b 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -6,11 +6,16 @@
*/
#include <linux/anon_inodes.h>
+#include <linux/atomic.h>
#include <linux/file.h>
#include <linux/fs.h>
+#include <linux/hrtimer.h>
+#include <linux/ktime.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
#include <linux/overflow.h>
+#include <linux/sched.h>
+#include <linux/sched/signal.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <uapi/linux/ntsync.h>
@@ -30,6 +35,8 @@ enum ntsync_type {
*
* Both rely on struct file for reference counting. Individual
* ntsync_obj objects take a reference to the device when created.
+ * Wait operations take a reference to each object being waited on for
+ * the duration of the wait.
*/
struct ntsync_obj {
@@ -47,12 +54,55 @@ struct ntsync_obj {
__u32 max;
} sem;
} u;
+
+ struct list_head any_waiters;
+};
+
+struct ntsync_q_entry {
+ struct list_head node;
+ struct ntsync_q *q;
+ struct ntsync_obj *obj;
+ __u32 index;
+};
+
+struct ntsync_q {
+ struct task_struct *task;
+
+ /*
+ * Protected via atomic_try_cmpxchg(). Only the thread that wins the
+ * compare-and-swap may actually change object states and wake this
+ * task.
+ */
+ atomic_t signaled;
+
+ __u32 count;
+ struct ntsync_q_entry entries[];
};
struct ntsync_device {
struct file *file;
};
+static void try_wake_any_sem(struct ntsync_obj *sem)
+{
+ struct ntsync_q_entry *entry;
+
+ lockdep_assert_held(&sem->lock);
+
+ list_for_each_entry(entry, &sem->any_waiters, node) {
+ struct ntsync_q *q = entry->q;
+ int signaled = -1;
+
+ if (!sem->u.sem.count)
+ break;
+
+ if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) {
+ sem->u.sem.count--;
+ wake_up_process(q->task);
+ }
+ }
+}
+
/*
* Actually change the semaphore state, returning -EOVERFLOW if it is made
* invalid.
@@ -88,6 +138,8 @@ static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
prev_count = sem->u.sem.count;
ret = post_sem_state(sem, args);
+ if (!ret)
+ try_wake_any_sem(sem);
spin_unlock(&sem->lock);
@@ -140,6 +192,7 @@ static struct ntsync_obj *ntsync_alloc_obj(struct ntsync_device *dev,
obj->dev = dev;
get_file(dev->file);
spin_lock_init(&obj->lock);
+ INIT_LIST_HEAD(&obj->any_waiters);
return obj;
}
@@ -190,6 +243,196 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
return put_user(fd, &user_args->sem);
}
+static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
+{
+ struct file *file = fget(fd);
+ struct ntsync_obj *obj;
+
+ if (!file)
+ return NULL;
+
+ if (file->f_op != &ntsync_obj_fops) {
+ fput(file);
+ return NULL;
+ }
+
+ obj = file->private_data;
+ if (obj->dev != dev) {
+ fput(file);
+ return NULL;
+ }
+
+ return obj;
+}
+
+static void put_obj(struct ntsync_obj *obj)
+{
+ fput(obj->file);
+}
+
+static int ntsync_schedule(const struct ntsync_q *q, const struct ntsync_wait_args *args)
+{
+ ktime_t timeout = ns_to_ktime(args->timeout);
+ clockid_t clock = CLOCK_MONOTONIC;
+ ktime_t *timeout_ptr;
+ int ret = 0;
+
+ timeout_ptr = (args->timeout == U64_MAX ? NULL : &timeout);
+
+ if (args->flags & NTSYNC_WAIT_REALTIME)
+ clock = CLOCK_REALTIME;
+
+ do {
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (atomic_read(&q->signaled) != -1) {
+ ret = 0;
+ break;
+ }
+ ret = schedule_hrtimeout_range_clock(timeout_ptr, 0, HRTIMER_MODE_ABS, clock);
+ } while (ret < 0);
+ __set_current_state(TASK_RUNNING);
+
+ return ret;
+}
+
+/*
+ * Allocate and initialize the ntsync_q structure, but do not queue us yet.
+ */
+static int setup_wait(struct ntsync_device *dev,
+ const struct ntsync_wait_args *args,
+ struct ntsync_q **ret_q)
+{
+ const __u32 count = args->count;
+ int fds[NTSYNC_MAX_WAIT_COUNT];
+ struct ntsync_q *q;
+ __u32 i, j;
+
+ if (args->pad[0] || args->pad[1] || args->pad[2] || (args->flags & ~NTSYNC_WAIT_REALTIME))
+ return -EINVAL;
+
+ if (args->count > NTSYNC_MAX_WAIT_COUNT)
+ return -EINVAL;
+
+ if (copy_from_user(fds, u64_to_user_ptr(args->objs),
+ array_size(count, sizeof(*fds))))
+ return -EFAULT;
+
+ q = kmalloc(struct_size(q, entries, count), GFP_KERNEL);
+ if (!q)
+ return -ENOMEM;
+ q->task = current;
+ atomic_set(&q->signaled, -1);
+ q->count = count;
+
+ for (i = 0; i < count; i++) {
+ struct ntsync_q_entry *entry = &q->entries[i];
+ struct ntsync_obj *obj = get_obj(dev, fds[i]);
+
+ if (!obj)
+ goto err;
+
+ entry->obj = obj;
+ entry->q = q;
+ entry->index = i;
+ }
+
+ *ret_q = q;
+ return 0;
+
+err:
+ for (j = 0; j < i; j++)
+ put_obj(q->entries[j].obj);
+ kfree(q);
+ return -EINVAL;
+}
+
+static void try_wake_any_obj(struct ntsync_obj *obj)
+{
+ switch (obj->type) {
+ case NTSYNC_TYPE_SEM:
+ try_wake_any_sem(obj);
+ break;
+ }
+}
+
+static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_wait_args args;
+ struct ntsync_q *q;
+ int signaled;
+ __u32 i;
+ int ret;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ ret = setup_wait(dev, &args, &q);
+ if (ret < 0)
+ return ret;
+
+ /* queue ourselves */
+
+ for (i = 0; i < args.count; i++) {
+ struct ntsync_q_entry *entry = &q->entries[i];
+ struct ntsync_obj *obj = entry->obj;
+
+ spin_lock(&obj->lock);
+ list_add_tail(&entry->node, &obj->any_waiters);
+ spin_unlock(&obj->lock);
+ }
+
+ /* check if we are already signaled */
+
+ for (i = 0; i < args.count; i++) {
+ struct ntsync_obj *obj = q->entries[i].obj;
+
+ if (atomic_read(&q->signaled) != -1)
+ break;
+
+ spin_lock(&obj->lock);
+ try_wake_any_obj(obj);
+ spin_unlock(&obj->lock);
+ }
+
+ /* sleep */
+
+ ret = ntsync_schedule(q, &args);
+
+ /* and finally, unqueue */
+
+ for (i = 0; i < args.count; i++) {
+ struct ntsync_q_entry *entry = &q->entries[i];
+ struct ntsync_obj *obj = entry->obj;
+
+ spin_lock(&obj->lock);
+ list_del(&entry->node);
+ spin_unlock(&obj->lock);
+
+ put_obj(obj);
+ }
+
+ signaled = atomic_read(&q->signaled);
+ if (signaled != -1) {
+ struct ntsync_wait_args __user *user_args = argp;
+
+ /* even if we caught a signal, we need to communicate success */
+ ret = 0;
+
+ if (put_user(signaled, &user_args->index))
+ ret = -EFAULT;
+ } else if (!ret) {
+ ret = -ETIMEDOUT;
+ }
+
+ kfree(q);
+ return ret;
+}
+
static int ntsync_char_open(struct inode *inode, struct file *file)
{
struct ntsync_device *dev;
@@ -221,6 +464,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case NTSYNC_IOC_CREATE_SEM:
return ntsync_create_sem(dev, argp);
+ case NTSYNC_IOC_WAIT_ANY:
+ return ntsync_wait_any(dev, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index dcfa38fdc93c..edc12c7a10dc 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -16,7 +16,21 @@ struct ntsync_sem_args {
__u32 max;
};
+#define NTSYNC_WAIT_REALTIME 0x1
+
+struct ntsync_wait_args {
+ __u64 timeout;
+ __u64 objs;
+ __u32 count;
+ __u32 index;
+ __u32 flags;
+ __u32 pad[3];
+};
+
+#define NTSYNC_MAX_WAIT_COUNT 64
+
#define NTSYNC_IOC_CREATE_SEM _IOWR('N', 0x80, struct ntsync_sem_args)
+#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v6 05/28] ntsync: Introduce NTSYNC_IOC_MUTEX_KILL.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This does not correspond to any NT syscall. Rather, when a thread dies, it
should be called by the NT emulator for each mutex, with the TID of the dying
thread.
NT mutexes are robust (in the pthread sense). When an NT thread dies, any
mutexes it owned are immediately released. Acquisition of those mutexes by other
threads will return a special value indicating that the mutex was abandoned,
like EOWNERDEAD returned from pthread_mutex_lock(), and EOWNERDEAD is indeed
used here for that purpose.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 61 +++++++++++++++++++++++++++++++++++--
include/uapi/linux/ntsync.h | 1 +
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 6ed278c17be9..0ed864d291a6 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -59,6 +59,7 @@ struct ntsync_obj {
struct {
__u32 count;
pid_t owner;
+ bool ownerdead;
} mutex;
} u;
@@ -107,6 +108,7 @@ struct ntsync_q {
atomic_t signaled;
bool all;
+ bool ownerdead;
__u32 count;
struct ntsync_q_entry entries[];
};
@@ -275,6 +277,9 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
obj->u.sem.count--;
break;
case NTSYNC_TYPE_MUTEX:
+ if (obj->u.mutex.ownerdead)
+ q->ownerdead = true;
+ obj->u.mutex.ownerdead = false;
obj->u.mutex.count++;
obj->u.mutex.owner = q->owner;
break;
@@ -338,6 +343,9 @@ static void try_wake_any_mutex(struct ntsync_obj *mutex)
continue;
if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) {
+ if (mutex->u.mutex.ownerdead)
+ q->ownerdead = true;
+ mutex->u.mutex.ownerdead = false;
mutex->u.mutex.count++;
mutex->u.mutex.owner = q->owner;
wake_up_process(q->task);
@@ -447,6 +455,52 @@ static int ntsync_mutex_unlock(struct ntsync_obj *mutex, void __user *argp)
return ret;
}
+/*
+ * Actually change the mutex state to mark its owner as dead,
+ * returning -EPERM if not the owner.
+ */
+static int kill_mutex_state(struct ntsync_obj *mutex, __u32 owner)
+{
+ ntsync_assert_held(mutex);
+
+ if (mutex->u.mutex.owner != owner)
+ return -EPERM;
+
+ mutex->u.mutex.ownerdead = true;
+ mutex->u.mutex.owner = 0;
+ mutex->u.mutex.count = 0;
+ return 0;
+}
+
+static int ntsync_mutex_kill(struct ntsync_obj *mutex, void __user *argp)
+{
+ struct ntsync_device *dev = mutex->dev;
+ __u32 owner;
+ bool all;
+ int ret;
+
+ if (get_user(owner, (__u32 __user *)argp))
+ return -EFAULT;
+ if (!owner)
+ return -EINVAL;
+
+ if (mutex->type != NTSYNC_TYPE_MUTEX)
+ return -EINVAL;
+
+ all = ntsync_lock_obj(dev, mutex);
+
+ ret = kill_mutex_state(mutex, owner);
+ if (!ret) {
+ if (all)
+ try_wake_all_obj(dev, mutex);
+ try_wake_any_mutex(mutex);
+ }
+
+ ntsync_unlock_obj(dev, mutex, all);
+
+ return ret;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -468,6 +522,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
return ntsync_sem_post(obj, argp);
case NTSYNC_IOC_MUTEX_UNLOCK:
return ntsync_mutex_unlock(obj, argp);
+ case NTSYNC_IOC_MUTEX_KILL:
+ return ntsync_mutex_kill(obj, argp);
default:
return -ENOIOCTLCMD;
}
@@ -658,6 +714,7 @@ static int setup_wait(struct ntsync_device *dev,
q->owner = args->owner;
atomic_set(&q->signaled, -1);
q->all = all;
+ q->ownerdead = false;
q->count = count;
for (i = 0; i < count; i++) {
@@ -766,7 +823,7 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
struct ntsync_wait_args __user *user_args = argp;
/* even if we caught a signal, we need to communicate success */
- ret = 0;
+ ret = q->ownerdead ? -EOWNERDEAD : 0;
if (put_user(signaled, &user_args->index))
ret = -EFAULT;
@@ -847,7 +904,7 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp)
struct ntsync_wait_args __user *user_args = argp;
/* even if we caught a signal, we need to communicate success */
- ret = 0;
+ ret = q->ownerdead ? -EOWNERDEAD : 0;
if (put_user(signaled, &user_args->index))
ret = -EFAULT;
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index a633db34f284..d7996180c1d2 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -43,5 +43,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
+#define NTSYNC_IOC_MUTEX_KILL _IOW ('N', 0x86, __u32)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 06/28] ntsync: Introduce NTSYNC_IOC_CREATE_EVENT.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This correspond to the NT syscall NtCreateEvent().
An NT event holds a single bit of state denoting whether it is signaled or
unsignaled.
There are two types of events: manual-reset and automatic-reset. When an
automatic-reset event is acquired via a wait function, its state is reset to
unsignaled. Manual-reset events are not affected by wait functions.
Whether the event is manual-reset, and its initial state, are specified at
creation time.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 62 +++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 7 +++++
2 files changed, 69 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 0ed864d291a6..ef8949ee2d4b 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -26,6 +26,7 @@
enum ntsync_type {
NTSYNC_TYPE_SEM,
NTSYNC_TYPE_MUTEX,
+ NTSYNC_TYPE_EVENT,
};
/*
@@ -61,6 +62,10 @@ struct ntsync_obj {
pid_t owner;
bool ownerdead;
} mutex;
+ struct {
+ bool manual;
+ bool signaled;
+ } event;
} u;
/*
@@ -233,6 +238,8 @@ static bool is_signaled(struct ntsync_obj *obj, __u32 owner)
if (obj->u.mutex.owner && obj->u.mutex.owner != owner)
return false;
return obj->u.mutex.count < UINT_MAX;
+ case NTSYNC_TYPE_EVENT:
+ return obj->u.event.signaled;
}
WARN(1, "bad object type %#x\n", obj->type);
@@ -283,6 +290,10 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
obj->u.mutex.count++;
obj->u.mutex.owner = q->owner;
break;
+ case NTSYNC_TYPE_EVENT:
+ if (!obj->u.event.manual)
+ obj->u.event.signaled = false;
+ break;
}
}
wake_up_process(q->task);
@@ -353,6 +364,28 @@ static void try_wake_any_mutex(struct ntsync_obj *mutex)
}
}
+static void try_wake_any_event(struct ntsync_obj *event)
+{
+ struct ntsync_q_entry *entry;
+
+ ntsync_assert_held(event);
+ lockdep_assert(event->type == NTSYNC_TYPE_EVENT);
+
+ list_for_each_entry(entry, &event->any_waiters, node) {
+ struct ntsync_q *q = entry->q;
+ int signaled = -1;
+
+ if (!event->u.event.signaled)
+ break;
+
+ if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) {
+ if (!event->u.event.manual)
+ event->u.event.signaled = false;
+ wake_up_process(q->task);
+ }
+ }
+}
+
/*
* Actually change the semaphore state, returning -EOVERFLOW if it is made
* invalid.
@@ -628,6 +661,30 @@ static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp)
return put_user(fd, &user_args->mutex);
}
+static int ntsync_create_event(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_event_args __user *user_args = argp;
+ struct ntsync_event_args args;
+ struct ntsync_obj *event;
+ int fd;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ event = ntsync_alloc_obj(dev, NTSYNC_TYPE_EVENT);
+ if (!event)
+ return -ENOMEM;
+ event->u.event.manual = args.manual;
+ event->u.event.signaled = args.signaled;
+ fd = ntsync_obj_get_fd(event);
+ if (fd < 0) {
+ kfree(event);
+ return fd;
+ }
+
+ return put_user(fd, &user_args->event);
+}
+
static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
{
struct file *file = fget(fd);
@@ -758,6 +815,9 @@ static void try_wake_any_obj(struct ntsync_obj *obj)
case NTSYNC_TYPE_MUTEX:
try_wake_any_mutex(obj);
break;
+ case NTSYNC_TYPE_EVENT:
+ try_wake_any_event(obj);
+ break;
}
}
@@ -947,6 +1007,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
void __user *argp = (void __user *)parm;
switch (cmd) {
+ case NTSYNC_IOC_CREATE_EVENT:
+ return ntsync_create_event(dev, argp);
case NTSYNC_IOC_CREATE_MUTEX:
return ntsync_create_mutex(dev, argp);
case NTSYNC_IOC_CREATE_SEM:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index d7996180c1d2..4c0c4271c7de 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -22,6 +22,12 @@ struct ntsync_mutex_args {
__u32 count;
};
+struct ntsync_event_args {
+ __u32 event;
+ __u32 manual;
+ __u32 signaled;
+};
+
#define NTSYNC_WAIT_REALTIME 0x1
struct ntsync_wait_args {
@@ -40,6 +46,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
#define NTSYNC_IOC_WAIT_ALL _IOWR('N', 0x83, struct ntsync_wait_args)
#define NTSYNC_IOC_CREATE_MUTEX _IOWR('N', 0x84, struct ntsync_sem_args)
+#define NTSYNC_IOC_CREATE_EVENT _IOWR('N', 0x87, struct ntsync_event_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
--
2.45.2
^ permalink raw reply related
* [PATCH v6 03/28] ntsync: Introduce NTSYNC_IOC_CREATE_MUTEX.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtCreateMutant().
An NT mutex is recursive, with a 32-bit recursion counter. When acquired via
NtWaitForMultipleObjects(), the recursion counter is incremented by one. The OS
records the thread which acquired it.
The OS records the thread which acquired it. However, in order to keep this
driver self-contained, the owning thread ID is managed by user-space, and passed
as a parameter to all relevant ioctls.
The initial owner and recursion count, if any, are specified when the mutex is
created.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 77 +++++++++++++++++++++++++++++++++++--
include/uapi/linux/ntsync.h | 10 ++++-
2 files changed, 83 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 0f4da571fa33..30c0bbb442da 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -25,6 +25,7 @@
enum ntsync_type {
NTSYNC_TYPE_SEM,
+ NTSYNC_TYPE_MUTEX,
};
/*
@@ -55,6 +56,10 @@ struct ntsync_obj {
__u32 count;
__u32 max;
} sem;
+ struct {
+ __u32 count;
+ pid_t owner;
+ } mutex;
} u;
/*
@@ -92,6 +97,7 @@ struct ntsync_q_entry {
struct ntsync_q {
struct task_struct *task;
+ __u32 owner;
/*
* Protected via atomic_try_cmpxchg(). Only the thread that wins the
@@ -214,13 +220,17 @@ static void ntsync_unlock_obj(struct ntsync_device *dev, struct ntsync_obj *obj,
((lockdep_is_held(&(obj)->dev->wait_all_lock) != LOCK_STATE_NOT_HELD) && \
(obj)->dev_locked))
-static bool is_signaled(struct ntsync_obj *obj)
+static bool is_signaled(struct ntsync_obj *obj, __u32 owner)
{
ntsync_assert_held(obj);
switch (obj->type) {
case NTSYNC_TYPE_SEM:
return !!obj->u.sem.count;
+ case NTSYNC_TYPE_MUTEX:
+ if (obj->u.mutex.owner && obj->u.mutex.owner != owner)
+ return false;
+ return obj->u.mutex.count < UINT_MAX;
}
WARN(1, "bad object type %#x\n", obj->type);
@@ -250,7 +260,7 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
}
for (i = 0; i < count; i++) {
- if (!is_signaled(q->entries[i].obj)) {
+ if (!is_signaled(q->entries[i].obj, q->owner)) {
can_wake = false;
break;
}
@@ -264,6 +274,10 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
case NTSYNC_TYPE_SEM:
obj->u.sem.count--;
break;
+ case NTSYNC_TYPE_MUTEX:
+ obj->u.mutex.count++;
+ obj->u.mutex.owner = q->owner;
+ break;
}
}
wake_up_process(q->task);
@@ -307,6 +321,30 @@ static void try_wake_any_sem(struct ntsync_obj *sem)
}
}
+static void try_wake_any_mutex(struct ntsync_obj *mutex)
+{
+ struct ntsync_q_entry *entry;
+
+ ntsync_assert_held(mutex);
+ lockdep_assert(mutex->type == NTSYNC_TYPE_MUTEX);
+
+ list_for_each_entry(entry, &mutex->any_waiters, node) {
+ struct ntsync_q *q = entry->q;
+ int signaled = -1;
+
+ if (mutex->u.mutex.count == UINT_MAX)
+ break;
+ if (mutex->u.mutex.owner && mutex->u.mutex.owner != q->owner)
+ continue;
+
+ if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) {
+ mutex->u.mutex.count++;
+ mutex->u.mutex.owner = q->owner;
+ wake_up_process(q->task);
+ }
+ }
+}
+
/*
* Actually change the semaphore state, returning -EOVERFLOW if it is made
* invalid.
@@ -454,6 +492,33 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
return put_user(fd, &user_args->sem);
}
+static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_mutex_args __user *user_args = argp;
+ struct ntsync_mutex_args args;
+ struct ntsync_obj *mutex;
+ int fd;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ if (!args.owner != !args.count)
+ return -EINVAL;
+
+ mutex = ntsync_alloc_obj(dev, NTSYNC_TYPE_MUTEX);
+ if (!mutex)
+ return -ENOMEM;
+ mutex->u.mutex.count = args.count;
+ mutex->u.mutex.owner = args.owner;
+ fd = ntsync_obj_get_fd(mutex);
+ if (fd < 0) {
+ kfree(mutex);
+ return fd;
+ }
+
+ return put_user(fd, &user_args->mutex);
+}
+
static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
{
struct file *file = fget(fd);
@@ -523,7 +588,7 @@ static int setup_wait(struct ntsync_device *dev,
struct ntsync_q *q;
__u32 i, j;
- if (args->pad[0] || args->pad[1] || args->pad[2] || (args->flags & ~NTSYNC_WAIT_REALTIME))
+ if (args->pad[0] || args->pad[1] || (args->flags & ~NTSYNC_WAIT_REALTIME))
return -EINVAL;
if (args->count > NTSYNC_MAX_WAIT_COUNT)
@@ -537,6 +602,7 @@ static int setup_wait(struct ntsync_device *dev,
if (!q)
return -ENOMEM;
q->task = current;
+ q->owner = args->owner;
atomic_set(&q->signaled, -1);
q->all = all;
q->count = count;
@@ -579,6 +645,9 @@ static void try_wake_any_obj(struct ntsync_obj *obj)
case NTSYNC_TYPE_SEM:
try_wake_any_sem(obj);
break;
+ case NTSYNC_TYPE_MUTEX:
+ try_wake_any_mutex(obj);
+ break;
}
}
@@ -768,6 +837,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
void __user *argp = (void __user *)parm;
switch (cmd) {
+ case NTSYNC_IOC_CREATE_MUTEX:
+ return ntsync_create_mutex(dev, argp);
case NTSYNC_IOC_CREATE_SEM:
return ntsync_create_sem(dev, argp);
case NTSYNC_IOC_WAIT_ALL:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index addf187b1573..d5e5a2fbcb4d 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -16,6 +16,12 @@ struct ntsync_sem_args {
__u32 max;
};
+struct ntsync_mutex_args {
+ __u32 mutex;
+ __u32 owner;
+ __u32 count;
+};
+
#define NTSYNC_WAIT_REALTIME 0x1
struct ntsync_wait_args {
@@ -24,7 +30,8 @@ struct ntsync_wait_args {
__u32 count;
__u32 index;
__u32 flags;
- __u32 pad[3];
+ __u32 owner;
+ __u32 pad[2];
};
#define NTSYNC_MAX_WAIT_COUNT 64
@@ -32,6 +39,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_CREATE_SEM _IOWR('N', 0x80, struct ntsync_sem_args)
#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
#define NTSYNC_IOC_WAIT_ALL _IOWR('N', 0x83, struct ntsync_wait_args)
+#define NTSYNC_IOC_CREATE_MUTEX _IOWR('N', 0x84, struct ntsync_sem_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v6 04/28] ntsync: Introduce NTSYNC_IOC_MUTEX_UNLOCK.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtReleaseMutant().
This syscall decrements the mutex's recursion count by one, and returns the
previous value. If the mutex is not owned by the current task, the function
instead fails and returns -EPERM.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 53 +++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 54 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 30c0bbb442da..6ed278c17be9 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -396,6 +396,57 @@ static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
return ret;
}
+/*
+ * Actually change the mutex state, returning -EPERM if not the owner.
+ */
+static int unlock_mutex_state(struct ntsync_obj *mutex,
+ const struct ntsync_mutex_args *args)
+{
+ ntsync_assert_held(mutex);
+
+ if (mutex->u.mutex.owner != args->owner)
+ return -EPERM;
+
+ if (!--mutex->u.mutex.count)
+ mutex->u.mutex.owner = 0;
+ return 0;
+}
+
+static int ntsync_mutex_unlock(struct ntsync_obj *mutex, void __user *argp)
+{
+ struct ntsync_mutex_args __user *user_args = argp;
+ struct ntsync_device *dev = mutex->dev;
+ struct ntsync_mutex_args args;
+ __u32 prev_count;
+ bool all;
+ int ret;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+ if (!args.owner)
+ return -EINVAL;
+
+ if (mutex->type != NTSYNC_TYPE_MUTEX)
+ return -EINVAL;
+
+ all = ntsync_lock_obj(dev, mutex);
+
+ prev_count = mutex->u.mutex.count;
+ ret = unlock_mutex_state(mutex, &args);
+ if (!ret) {
+ if (all)
+ try_wake_all_obj(dev, mutex);
+ try_wake_any_mutex(mutex);
+ }
+
+ ntsync_unlock_obj(dev, mutex, all);
+
+ if (!ret && put_user(prev_count, &user_args->count))
+ ret = -EFAULT;
+
+ return ret;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -415,6 +466,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case NTSYNC_IOC_SEM_POST:
return ntsync_sem_post(obj, argp);
+ case NTSYNC_IOC_MUTEX_UNLOCK:
+ return ntsync_mutex_unlock(obj, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index d5e5a2fbcb4d..a633db34f284 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -42,5 +42,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_CREATE_MUTEX _IOWR('N', 0x84, struct ntsync_sem_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
+#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 10/28] ntsync: Introduce NTSYNC_IOC_SEM_READ.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtQuerySemaphore().
This returns the current count and maximum count of the semaphore.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 26 ++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 6b8352270874..d6e8a4bde1d0 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -583,6 +583,30 @@ static int ntsync_event_reset(struct ntsync_obj *event, void __user *argp)
return 0;
}
+static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp)
+{
+ struct ntsync_sem_args __user *user_args = argp;
+ struct ntsync_device *dev = sem->dev;
+ struct ntsync_sem_args args;
+ bool all;
+
+ if (sem->type != NTSYNC_TYPE_SEM)
+ return -EINVAL;
+
+ args.sem = 0;
+
+ all = ntsync_lock_obj(dev, sem);
+
+ args.count = sem->u.sem.count;
+ args.max = sem->u.sem.max;
+
+ ntsync_unlock_obj(dev, sem, all);
+
+ if (copy_to_user(user_args, &args, sizeof(args)))
+ return -EFAULT;
+ return 0;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -602,6 +626,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case NTSYNC_IOC_SEM_POST:
return ntsync_sem_post(obj, argp);
+ case NTSYNC_IOC_SEM_READ:
+ return ntsync_sem_read(obj, argp);
case NTSYNC_IOC_MUTEX_UNLOCK:
return ntsync_mutex_unlock(obj, argp);
case NTSYNC_IOC_MUTEX_KILL:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 5586fadd9bdd..5e922703686f 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -54,5 +54,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_EVENT_SET _IOR ('N', 0x88, __u32)
#define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32)
#define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32)
+#define NTSYNC_IOC_SEM_READ _IOR ('N', 0x8b, struct ntsync_sem_args)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 08/28] ntsync: Introduce NTSYNC_IOC_EVENT_RESET.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtResetEvent().
This sets the event to the unsignaled state, and returns its previous state.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 24 ++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 25 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index e46dd795370a..092133699193 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -559,6 +559,28 @@ static int ntsync_event_set(struct ntsync_obj *event, void __user *argp)
return 0;
}
+static int ntsync_event_reset(struct ntsync_obj *event, void __user *argp)
+{
+ struct ntsync_device *dev = event->dev;
+ __u32 prev_state;
+ bool all;
+
+ if (event->type != NTSYNC_TYPE_EVENT)
+ return -EINVAL;
+
+ all = ntsync_lock_obj(dev, event);
+
+ prev_state = event->u.event.signaled;
+ event->u.event.signaled = false;
+
+ ntsync_unlock_obj(dev, event, all);
+
+ if (put_user(prev_state, (__u32 __user *)argp))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -584,6 +606,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
return ntsync_mutex_kill(obj, argp);
case NTSYNC_IOC_EVENT_SET:
return ntsync_event_set(obj, argp);
+ case NTSYNC_IOC_EVENT_RESET:
+ return ntsync_event_reset(obj, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 36d903521bbe..7fdf79729b20 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -52,5 +52,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
#define NTSYNC_IOC_MUTEX_KILL _IOW ('N', 0x86, __u32)
#define NTSYNC_IOC_EVENT_SET _IOR ('N', 0x88, __u32)
+#define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 09/28] ntsync: Introduce NTSYNC_IOC_EVENT_PULSE.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtPulseEvent().
This wakes up any waiters as if the event had been set, but does not set the
event, instead resetting it if it had been signalled. Thus, for a manual-reset
event, all waiters are woken, whereas for an auto-reset event, at most one
waiter is woken.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 8 ++++++--
include/uapi/linux/ntsync.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 092133699193..6b8352270874 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -534,7 +534,7 @@ static int ntsync_mutex_kill(struct ntsync_obj *mutex, void __user *argp)
return ret;
}
-static int ntsync_event_set(struct ntsync_obj *event, void __user *argp)
+static int ntsync_event_set(struct ntsync_obj *event, void __user *argp, bool pulse)
{
struct ntsync_device *dev = event->dev;
__u32 prev_state;
@@ -550,6 +550,8 @@ static int ntsync_event_set(struct ntsync_obj *event, void __user *argp)
if (all)
try_wake_all_obj(dev, event);
try_wake_any_event(event);
+ if (pulse)
+ event->u.event.signaled = false;
ntsync_unlock_obj(dev, event, all);
@@ -605,9 +607,11 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
case NTSYNC_IOC_MUTEX_KILL:
return ntsync_mutex_kill(obj, argp);
case NTSYNC_IOC_EVENT_SET:
- return ntsync_event_set(obj, argp);
+ return ntsync_event_set(obj, argp, false);
case NTSYNC_IOC_EVENT_RESET:
return ntsync_event_reset(obj, argp);
+ case NTSYNC_IOC_EVENT_PULSE:
+ return ntsync_event_set(obj, argp, true);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 7fdf79729b20..5586fadd9bdd 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -53,5 +53,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_MUTEX_KILL _IOW ('N', 0x86, __u32)
#define NTSYNC_IOC_EVENT_SET _IOR ('N', 0x88, __u32)
#define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32)
+#define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 15/28] selftests: ntsync: Add some tests for mutex state.
From: Elizabeth Figura @ 2024-12-09 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Test mutex-specific ioctls NTSYNC_IOC_MUTEX_UNLOCK and NTSYNC_IOC_MUTEX_READ,
and waiting on mutexes.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 196 ++++++++++++++++++
1 file changed, 196 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 1e145c6dfded..7cd0f40594fd 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -40,6 +40,39 @@ static int post_sem(int sem, __u32 *count)
return ioctl(sem, NTSYNC_IOC_SEM_POST, count);
}
+static int read_mutex_state(int mutex, __u32 *count, __u32 *owner)
+{
+ struct ntsync_mutex_args args;
+ int ret;
+
+ memset(&args, 0xcc, sizeof(args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &args);
+ *count = args.count;
+ *owner = args.owner;
+ return ret;
+}
+
+#define check_mutex_state(mutex, count, owner) \
+ ({ \
+ __u32 __count, __owner; \
+ int ret = read_mutex_state((mutex), &__count, &__owner); \
+ EXPECT_EQ(0, ret); \
+ EXPECT_EQ((count), __count); \
+ EXPECT_EQ((owner), __owner); \
+ })
+
+static int unlock_mutex(int mutex, __u32 owner, __u32 *count)
+{
+ struct ntsync_mutex_args args;
+ int ret;
+
+ args.owner = owner;
+ args.count = 0xdeadbeef;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_UNLOCK, &args);
+ *count = args.count;
+ return ret;
+}
+
static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
{
struct ntsync_wait_args args = {0};
@@ -146,4 +179,167 @@ TEST(semaphore_state)
close(fd);
}
+TEST(mutex_state)
+{
+ struct ntsync_mutex_args mutex_args;
+ __u32 owner, count, index;
+ struct timespec timeout;
+ int fd, ret, mutex;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ mutex_args.owner = 123;
+ mutex_args.count = 0;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ mutex_args.owner = 0;
+ mutex_args.count = 2;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ mutex_args.owner = 123;
+ mutex_args.count = 2;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, 2, 123);
+
+ ret = unlock_mutex(mutex, 0, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ ret = unlock_mutex(mutex, 456, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+ check_mutex_state(mutex, 2, 123);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+ check_mutex_state(mutex, 1, 123);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, count);
+ check_mutex_state(mutex, 0, 0);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+
+ ret = wait_any(fd, 1, &mutex, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 456);
+
+ ret = wait_any(fd, 1, &mutex, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 2, 456);
+
+ ret = unlock_mutex(mutex, 456, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+ check_mutex_state(mutex, 1, 456);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ owner = 0;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ owner = 123;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+ check_mutex_state(mutex, 1, 456);
+
+ owner = 456;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ owner = 123;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ close(mutex);
+
+ mutex_args.owner = 0;
+ mutex_args.count = 0;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, 0, 0);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ close(mutex);
+
+ mutex_args.owner = 123;
+ mutex_args.count = ~0u;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, ~0u, 123);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ close(mutex);
+
+ close(fd);
+}
+
TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox