* 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 v6 28/28] ntsync: No longer depend on BROKEN.
From: kernel test robot @ 2024-12-12 4:52 UTC (permalink / raw)
To: Elizabeth Figura, Arnd Bergmann, Greg Kroah-Hartman,
Jonathan Corbet, Shuah Khan
Cc: oe-kbuild-all, 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-29-zfigura@codeweavers.com>
Hi Elizabeth,
kernel test robot noticed the following build errors:
[auto build test ERROR on cdd30ebb1b9f36159d66f088b61aee264e649d7a]
url: https://github.com/intel-lab-lkp/linux/commits/Elizabeth-Figura/ntsync-Introduce-NTSYNC_IOC_WAIT_ANY/20241210-031155
base: cdd30ebb1b9f36159d66f088b61aee264e649d7a
patch link: https://lore.kernel.org/r/20241209185904.507350-29-zfigura%40codeweavers.com
patch subject: [PATCH v6 28/28] ntsync: No longer depend on BROKEN.
config: i386-randconfig-002-20241212 (https://download.01.org/0day-ci/archive/20241212/202412121219.EQhUbN0S-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241212/202412121219.EQhUbN0S-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412121219.EQhUbN0S-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/spinlock.h:60,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from drivers/misc/ntsync.c:11:
In function 'check_copy_size',
inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
inlined from 'setup_wait' at drivers/misc/ntsync.c:903:6:
>> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
259 | __bad_copy_to();
| ^~~~~~~~~~~~~~~
vim +/__bad_copy_to +259 include/linux/thread_info.h
b0377fedb652808 Al Viro 2017-06-29 248
9dd819a15162f8f Kees Cook 2019-09-25 249 static __always_inline __must_check bool
b0377fedb652808 Al Viro 2017-06-29 250 check_copy_size(const void *addr, size_t bytes, bool is_source)
b0377fedb652808 Al Viro 2017-06-29 251 {
c80d92fbb67b2c8 Kees Cook 2021-06-17 252 int sz = __builtin_object_size(addr, 0);
b0377fedb652808 Al Viro 2017-06-29 253 if (unlikely(sz >= 0 && sz < bytes)) {
b0377fedb652808 Al Viro 2017-06-29 254 if (!__builtin_constant_p(bytes))
b0377fedb652808 Al Viro 2017-06-29 255 copy_overflow(sz, bytes);
b0377fedb652808 Al Viro 2017-06-29 256 else if (is_source)
b0377fedb652808 Al Viro 2017-06-29 257 __bad_copy_from();
b0377fedb652808 Al Viro 2017-06-29 258 else
b0377fedb652808 Al Viro 2017-06-29 @259 __bad_copy_to();
b0377fedb652808 Al Viro 2017-06-29 260 return false;
b0377fedb652808 Al Viro 2017-06-29 261 }
6d13de1489b6bf5 Kees Cook 2019-12-04 262 if (WARN_ON_ONCE(bytes > INT_MAX))
6d13de1489b6bf5 Kees Cook 2019-12-04 263 return false;
b0377fedb652808 Al Viro 2017-06-29 264 check_object_size(addr, bytes, is_source);
b0377fedb652808 Al Viro 2017-06-29 265 return true;
b0377fedb652808 Al Viro 2017-06-29 266 }
b0377fedb652808 Al Viro 2017-06-29 267
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v6 28/28] ntsync: No longer depend on BROKEN.
From: Arnd Bergmann @ 2024-12-12 7:18 UTC (permalink / raw)
To: kernel test robot, Elizabeth Figura, Greg Kroah-Hartman,
Jonathan Corbet, Shuah Khan
Cc: oe-kbuild-all, 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: <202412121219.EQhUbN0S-lkp@intel.com>
On Thu, Dec 12, 2024, at 05:52, kernel test robot wrote:
> Hi Elizabeth,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on cdd30ebb1b9f36159d66f088b61aee264e649d7a]
>
> url:
> https://github.com/intel-lab-lkp/linux/commits/Elizabeth-Figura/ntsync-Introduce-NTSYNC_IOC_WAIT_ANY/20241210-031155
> base: cdd30ebb1b9f36159d66f088b61aee264e649d7a
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/spinlock.h:60,
> from include/linux/wait.h:9,
> from include/linux/wait_bit.h:8,
> from include/linux/fs.h:6,
> from drivers/misc/ntsync.c:11:
> In function 'check_copy_size',
> inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
> inlined from 'setup_wait' at drivers/misc/ntsync.c:903:6:
>>> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
> 259 | __bad_copy_to();
> | ^~~~~~~~~~~~~~~
I looked up the function from the github URL above and found
int fds[NTSYNC_MAX_WAIT_COUNT + 1];
const __u32 count = args->count;
struct ntsync_q *q;
__u32 total_count;
__u32 i, j;
if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME))
return -EINVAL;
if (args->count > NTSYNC_MAX_WAIT_COUNT)
return -EINVAL;
total_count = count;
if (args->alert)
total_count++;
if (copy_from_user(fds, u64_to_user_ptr(args->objs),
array_size(count, sizeof(*fds))))
return -EFAULT;
which looks correct to me, as it has appropriate
range checking on args->count, but I can see how
the warning may be a result of checking 'args->count'
instead of 'count'.
Arnd
^ permalink raw reply
* Re: [PATCH v6 00/28] NT synchronization primitive driver
From: Peter Zijlstra @ 2024-12-12 12:01 UTC (permalink / raw)
To: Elizabeth Figura
Cc: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, 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 09, 2024 at 12:58:36PM -0600, Elizabeth Figura wrote:
> 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.
I see no problem making those changes; esp. since Arnd doesn't seem to
object to the latter.
> 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.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* [PATCH v23 0/8] Script execution control (was O_MAYEXEC)
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
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 rename of
AUDIT_INTEGRITY_DATA_CHECK to AUDIT_INTEGRITY_USERSPACE, and new
Reviewed-by and Tested-by tags. This series is based on v6.13-rc2 and
it also applies on linux-next.
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, this series should be good now, I'll let you take it to your tree
if it's OK with you.
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.
[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/20241212174223.389435-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
-----------------
v22: https://lore.kernel.org/r/20241205160925.230119-1-mic@digikod.net
v21: https://lore.kernel.org/r/20241112191858.162021-1-mic@digikod.net
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: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
--
2.47.1
^ permalink raw reply
* [PATCH v23 1/8] exec: Add a new AT_EXECVE_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Jeff Xu
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
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>
Reviewed-by: Jeff Xu <jeffxu@chromium.org>
Tested-by: Jeff Xu <jeffxu@chromium.org>
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/20241212174223.389435-2-mic@digikod.net
---
Changes since v21:
* Remove the audit changes, requested by Paul.
* Add Paul's Acked-by.
* Fix a typo in comments and in commit message.
* Add SPDX-License-Identifier header to the documentation.
* Rebase on v6.13-rc1 .
* Add Jeff's Reviewed-by and Tested-by.
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 Serge's Reviewed-by.
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
^ permalink raw reply related
* [PATCH v23 2/8] security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Andy Lutomirski, Jeff Xu
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
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>
Reviewed-by: Jeff Xu <jeffxu@chromium.org>
Tested-by: Jeff Xu <jeffxu@chromium.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-3-mic@digikod.net
---
Changes since v22:
* Add Jeff's Reviewed-by and Tested-by.
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.
* Add Serge's Reviewed-by.
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).
+
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
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d98877ff1a..3fba30dbd68b 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,32 @@
#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
+/* See Documentation/userspace-api/check_exec.rst */
+#define SECURE_EXEC_RESTRICT_FILE 8
+#define SECURE_EXEC_RESTRICT_FILE_LOCKED 9 /* make bit-8 immutable */
+
+#define SECBIT_EXEC_RESTRICT_FILE (issecure_mask(SECURE_EXEC_RESTRICT_FILE))
+#define SECBIT_EXEC_RESTRICT_FILE_LOCKED \
+ (issecure_mask(SECURE_EXEC_RESTRICT_FILE_LOCKED))
+
+/* See Documentation/userspace-api/check_exec.rst */
+#define SECURE_EXEC_DENY_INTERACTIVE 10
+#define SECURE_EXEC_DENY_INTERACTIVE_LOCKED 11 /* make bit-10 immutable */
+
+#define SECBIT_EXEC_DENY_INTERACTIVE \
+ (issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
+#define SECBIT_EXEC_DENY_INTERACTIVE_LOCKED \
+ (issecure_mask(SECURE_EXEC_DENY_INTERACTIVE_LOCKED))
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
issecure_mask(SECURE_KEEP_CAPS) | \
- issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+ issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+ issecure_mask(SECURE_EXEC_RESTRICT_FILE) | \
+ issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
+#define SECURE_ALL_UNPRIVILEGED (issecure_mask(SECURE_EXEC_RESTRICT_FILE) | \
+ issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
+
#endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index cefad323a0b1..52ea01acb453 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1302,21 +1302,38 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (old->securebits ^ arg2)) /*[1]*/
|| ((old->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current_cred(),
- current_cred()->user_ns,
- CAP_SETPCAP,
- CAP_OPT_NONE) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
* [3] no setting of unsupported bits
- * [4] doing anything requires privilege (go read about
- * the "sendmail capabilities bug")
*/
)
/* cannot change a locked bit */
return -EPERM;
+ /*
+ * Doing anything requires privilege (go read about the
+ * "sendmail capabilities bug"), except for unprivileged bits.
+ * Indeed, the SECURE_ALL_UNPRIVILEGED bits are not
+ * restrictions enforced by the kernel but by user space on
+ * itself.
+ */
+ if (cap_capable(current_cred(), current_cred()->user_ns,
+ CAP_SETPCAP, CAP_OPT_NONE) != 0) {
+ const unsigned long unpriv_and_locks =
+ SECURE_ALL_UNPRIVILEGED |
+ SECURE_ALL_UNPRIVILEGED << 1;
+ const unsigned long changed = old->securebits ^ arg2;
+
+ /* For legacy reason, denies non-change. */
+ if (!changed)
+ return -EPERM;
+
+ /* Denies privileged changes. */
+ if (changed & ~unpriv_and_locks)
+ return -EPERM;
+ }
+
new = prepare_creds();
if (!new)
return -ENOMEM;
--
2.47.1
^ permalink raw reply related
* [PATCH v23 3/8] selftests/exec: Add 32 tests for AT_EXECVE_CHECK and exec securebits
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
Test that checks performed by execveat(..., AT_EXECVE_CHECK) are
consistent with noexec mount points and file execute permissions.
Test that SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE are
inherited by child processes and that they can be pinned with the
appropriate SECBIT_EXEC_RESTRICT_FILE_LOCKED and
SECBIT_EXEC_DENY_INTERACTIVE_LOCKED bits.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-4-mic@digikod.net
---
Changes since v21:
* Fix test's variants formatting.
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
Changes since v19:
* Rename securebits.
* Rename test file.
Changes since v18:
* Rewrite tests with the new design: execveat/AT_CHECK and securebits.
* Simplify the capability dropping and improve it with the NOROOT
securebits.
* Replace most ASSERT with EXPECT.
* Fix NULL execve's argv to avoid kernel warning.
* Move tests to exec/
* Build a "false" static binary to test full execution path.
Changes since v14:
* Add Reviewed-by Kees Cook.
Changes since v13:
* Move -I to CFLAGS (suggested by Kees Cook).
* Update sysctl name.
Changes since v12:
* Fix Makefile's license.
Changes since v10:
* Update selftest Makefile.
Changes since v9:
* Rename the syscall and the sysctl.
* Update tests for enum trusted_for_usage
Changes since v8:
* Update with the dedicated syscall introspect_access(2) and the renamed
fs.introspection_policy sysctl.
* Remove check symlink which can't be use as is anymore.
* Use socketpair(2) to test UNIX socket.
Changes since v7:
* Update tests with faccessat2/AT_INTERPRETED, including new ones to
check that setting R_OK or W_OK returns EINVAL.
* Add tests for memfd, pipefs and nsfs.
* Rename and move back tests to a standalone directory.
Changes since v6:
* Add full combination tests for all file types, including block
devices, character devices, fifos, sockets and symlinks.
* Properly save and restore initial sysctl value for all tests.
Changes since v5:
* Refactor with FIXTURE_VARIANT, which make the tests much more easy to
read and maintain.
* Save and restore initial sysctl value (suggested by Kees Cook).
* Test with a sysctl value of 0.
* Check errno in sysctl_access_write test.
* Update tests for the CAP_SYS_ADMIN switch.
* Update tests to check -EISDIR (replacing -EACCES).
* Replace FIXTURE_DATA() with FIXTURE() (spotted by Kees Cook).
* Use global const strings.
Changes since v3:
* Replace RESOLVE_MAYEXEC with O_MAYEXEC.
* Add tests to check that O_MAYEXEC is ignored by open(2) and openat(2).
Changes since v2:
* Move tests from exec/ to openat2/ .
* Replace O_MAYEXEC with RESOLVE_MAYEXEC from openat2(2).
* Cleanup tests.
Changes since v1:
* Move tests from yama/ to exec/ .
* Fix _GNU_SOURCE in kselftest_harness.h .
* Add a new test sysctl_access_write to check if CAP_MAC_ADMIN is taken
into account.
* Test directory execution which is always forbidden since commit
73601ea5b7b1 ("fs/open.c: allow opening only regular files during
execve()"), and also check that even the root user can not bypass file
execution checks.
* Make sure delete_workspace() always as enough right to succeed.
* Cosmetic cleanup.
---
tools/testing/selftests/exec/.gitignore | 2 +
tools/testing/selftests/exec/Makefile | 7 +
tools/testing/selftests/exec/check-exec.c | 456 ++++++++++++++++++++++
tools/testing/selftests/exec/config | 2 +
tools/testing/selftests/exec/false.c | 5 +
5 files changed, 472 insertions(+)
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
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index a0dc5d4bf733..a32c63bb4df1 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -9,6 +9,8 @@ execveat.ephemeral
execveat.denatured
non-regular
null-argv
+/check-exec
+/false
/load_address.*
!load_address.c
/recursion-depth
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index ba012bc5aab9..8713d1c862ae 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS = -Wall
CFLAGS += -Wno-nonnull
+CFLAGS += $(KHDR_INCLUDES)
+
+LDLIBS += -lcap
ALIGNS := 0x1000 0x200000 0x1000000
ALIGN_PIES := $(patsubst %,load_address.%,$(ALIGNS))
@@ -9,12 +12,14 @@ ALIGNMENT_TESTS := $(ALIGN_PIES) $(ALIGN_STATIC_PIES)
TEST_PROGS := binfmt_script.py
TEST_GEN_PROGS := execveat non-regular $(ALIGNMENT_TESTS)
+TEST_GEN_PROGS_EXTENDED := false
TEST_GEN_FILES := execveat.symlink execveat.denatured script subdir
# Makefile is a run-time dependency, since it's accessed by the execveat test
TEST_FILES := Makefile
TEST_GEN_PROGS += recursion-depth
TEST_GEN_PROGS += null-argv
+TEST_GEN_PROGS += check-exec
EXTRA_CLEAN := $(OUTPUT)/subdir.moved $(OUTPUT)/execveat.moved $(OUTPUT)/xxxxx* \
$(OUTPUT)/S_I*.test
@@ -38,3 +43,5 @@ $(OUTPUT)/load_address.0x%: load_address.c
$(OUTPUT)/load_address.static.0x%: load_address.c
$(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=$(lastword $(subst ., ,$@)) \
-fPIE -static-pie $< -o $@
+$(OUTPUT)/false: false.c
+ $(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
diff --git a/tools/testing/selftests/exec/check-exec.c b/tools/testing/selftests/exec/check-exec.c
new file mode 100644
index 000000000000..4d3f4525e1e1
--- /dev/null
+++ b/tools/testing/selftests/exec/check-exec.c
@@ -0,0 +1,456 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test execveat(2) with AT_EXECVE_CHECK, and prctl(2) with
+ * SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE, and their locked
+ * counterparts.
+ *
+ * Copyright © 2018-2020 ANSSI
+ * Copyright © 2024 Microsoft Corporation
+ *
+ * Author: Mickaël Salaün <mic@digikod.net>
+ */
+
+#include <asm-generic/unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/capability.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <unistd.h>
+
+/* Defines AT_EXECVE_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
+#include "../kselftest_harness.h"
+
+static void drop_privileges(struct __test_metadata *const _metadata)
+{
+ const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED;
+ cap_t cap_p;
+
+ if ((cap_get_secbits() & noroot) != noroot)
+ EXPECT_EQ(0, cap_set_secbits(noroot));
+
+ cap_p = cap_get_proc();
+ EXPECT_NE(NULL, cap_p);
+ EXPECT_NE(-1, cap_clear(cap_p));
+
+ /*
+ * Drops everything, especially CAP_SETPCAP, CAP_DAC_OVERRIDE, and
+ * CAP_DAC_READ_SEARCH.
+ */
+ EXPECT_NE(-1, cap_set_proc(cap_p));
+ EXPECT_NE(-1, cap_free(cap_p));
+}
+
+static int test_secbits_set(const unsigned int secbits)
+{
+ int err;
+
+ err = prctl(PR_SET_SECUREBITS, secbits);
+ if (err)
+ return errno;
+ return 0;
+}
+
+FIXTURE(access)
+{
+ int memfd, pipefd;
+ int pipe_fds[2], socket_fds[2];
+};
+
+FIXTURE_VARIANT(access)
+{
+ const bool mount_exec;
+ const bool file_exec;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(access, mount_exec_file_exec) {
+ /* clang-format on */
+ .mount_exec = true,
+ .file_exec = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(access, mount_exec_file_noexec) {
+ /* clang-format on */
+ .mount_exec = true,
+ .file_exec = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_exec) {
+ /* clang-format on */
+ .mount_exec = false,
+ .file_exec = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_noexec) {
+ /* clang-format on */
+ .mount_exec = false,
+ .file_exec = false,
+};
+
+static const char binary_path[] = "./false";
+static const char workdir_path[] = "./test-mount";
+static const char reg_file_path[] = "./test-mount/regular_file";
+static const char dir_path[] = "./test-mount/directory";
+static const char block_dev_path[] = "./test-mount/block_device";
+static const char char_dev_path[] = "./test-mount/character_device";
+static const char fifo_path[] = "./test-mount/fifo";
+
+FIXTURE_SETUP(access)
+{
+ int procfd_path_size;
+ static const char path_template[] = "/proc/self/fd/%d";
+ char procfd_path[sizeof(path_template) + 10];
+
+ /* Makes sure we are not already restricted nor locked. */
+ EXPECT_EQ(0, test_secbits_set(0));
+
+ /*
+ * Cleans previous workspace if any error previously happened (don't
+ * check errors).
+ */
+ umount(workdir_path);
+ rmdir(workdir_path);
+
+ /* Creates a clean mount point. */
+ ASSERT_EQ(0, mkdir(workdir_path, 00700));
+ ASSERT_EQ(0, mount("test", workdir_path, "tmpfs",
+ MS_MGC_VAL | (variant->mount_exec ? 0 : MS_NOEXEC),
+ "mode=0700,size=9m"));
+
+ /* Creates a regular file. */
+ ASSERT_EQ(0, mknod(reg_file_path,
+ S_IFREG | (variant->file_exec ? 0700 : 0600), 0));
+ /* Creates a directory. */
+ ASSERT_EQ(0, mkdir(dir_path, variant->file_exec ? 0700 : 0600));
+ /* Creates a character device: /dev/null. */
+ ASSERT_EQ(0, mknod(char_dev_path, S_IFCHR | 0400, makedev(1, 3)));
+ /* Creates a block device: /dev/loop0 */
+ ASSERT_EQ(0, mknod(block_dev_path, S_IFBLK | 0400, makedev(7, 0)));
+ /* Creates a fifo. */
+ ASSERT_EQ(0, mknod(fifo_path, S_IFIFO | 0600, 0));
+
+ /* Creates a regular file without user mount point. */
+ self->memfd = memfd_create("test-exec-probe", MFD_CLOEXEC);
+ ASSERT_LE(0, self->memfd);
+ /* Sets mode, which must be ignored by the exec check. */
+ ASSERT_EQ(0, fchmod(self->memfd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a pipefs file descriptor. */
+ ASSERT_EQ(0, pipe(self->pipe_fds));
+ procfd_path_size = snprintf(procfd_path, sizeof(procfd_path),
+ path_template, self->pipe_fds[0]);
+ ASSERT_LT(procfd_path_size, sizeof(procfd_path));
+ self->pipefd = open(procfd_path, O_RDWR | O_CLOEXEC);
+ ASSERT_LE(0, self->pipefd);
+ ASSERT_EQ(0, fchmod(self->pipefd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a socket file descriptor. */
+ ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0,
+ self->socket_fds));
+}
+
+FIXTURE_TEARDOWN_PARENT(access)
+{
+ /* There is no need to unlink the test files. */
+ EXPECT_EQ(0, umount(workdir_path));
+ EXPECT_EQ(0, rmdir(workdir_path));
+}
+
+static void fill_exec_fd(struct __test_metadata *_metadata, const int fd_out)
+{
+ char buf[1024];
+ size_t len;
+ int fd_in;
+
+ fd_in = open(binary_path, O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd_in);
+ /* Cannot use copy_file_range(2) because of EXDEV. */
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ while (len > 0) {
+ EXPECT_EQ(len, write(fd_out, buf, len))
+ {
+ TH_LOG("Failed to write: %s (%d)", strerror(errno),
+ errno);
+ }
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ }
+ EXPECT_EQ(0, close(fd_in));
+}
+
+static void fill_exec_path(struct __test_metadata *_metadata,
+ const char *const path)
+{
+ int fd_out;
+
+ fd_out = open(path, O_CLOEXEC | O_WRONLY);
+ ASSERT_LE(0, fd_out)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ fill_exec_fd(_metadata, fd_out);
+ EXPECT_EQ(0, close(fd_out));
+}
+
+static void test_exec_fd(struct __test_metadata *_metadata, const int fd,
+ const int err_code)
+{
+ char *const argv[] = { "", NULL };
+ int access_ret, access_errno;
+
+ /*
+ * If we really execute fd, filled with the "false" binary, the current
+ * thread will exits with an error, which will be interpreted by the
+ * test framework as an error. With AT_EXECVE_CHECK, we only check a
+ * potential successful execution.
+ */
+ access_ret =
+ execveat(fd, "", argv, NULL, AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ access_errno = errno;
+ if (err_code) {
+ EXPECT_EQ(-1, access_ret);
+ EXPECT_EQ(err_code, access_errno)
+ {
+ TH_LOG("Wrong error for execveat(2): %s (%d)",
+ strerror(access_errno), errno);
+ }
+ } else {
+ EXPECT_EQ(0, access_ret)
+ {
+ TH_LOG("Access denied: %s", strerror(access_errno));
+ }
+ }
+}
+
+static void test_exec_path(struct __test_metadata *_metadata,
+ const char *const path, const int err_code)
+{
+ int flags = O_CLOEXEC;
+ int fd;
+
+ /* Do not block on pipes. */
+ if (path == fifo_path)
+ flags |= O_NONBLOCK;
+
+ fd = open(path, flags | O_RDONLY);
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ test_exec_fd(_metadata, fd, err_code);
+ EXPECT_EQ(0, close(fd));
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, regular_file_empty)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+TEST_F(access, regular_file_elf)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ fill_exec_path(_metadata, reg_file_path);
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, memfd_empty)
+{
+ const int exec = variant->file_exec;
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, memfd_elf)
+{
+ const int exec = variant->file_exec;
+
+ fill_exec_fd(_metadata, self->memfd);
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, non_regular_files)
+{
+ test_exec_path(_metadata, dir_path, EACCES);
+ test_exec_path(_metadata, block_dev_path, EACCES);
+ test_exec_path(_metadata, char_dev_path, EACCES);
+ test_exec_path(_metadata, fifo_path, EACCES);
+ test_exec_fd(_metadata, self->socket_fds[0], EACCES);
+ test_exec_fd(_metadata, self->pipefd, EACCES);
+}
+
+/* clang-format off */
+FIXTURE(secbits) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(secbits)
+{
+ const bool is_privileged;
+ const int error;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, priv) {
+ /* clang-format on */
+ .is_privileged = true,
+ .error = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, unpriv) {
+ /* clang-format on */
+ .is_privileged = false,
+ .error = EPERM,
+};
+
+FIXTURE_SETUP(secbits)
+{
+ /* Makes sure no exec bits are set. */
+ EXPECT_EQ(0, test_secbits_set(0));
+ EXPECT_EQ(0, prctl(PR_GET_SECUREBITS));
+
+ if (!variant->is_privileged)
+ drop_privileges(_metadata);
+}
+
+FIXTURE_TEARDOWN(secbits)
+{
+}
+
+TEST_F(secbits, legacy)
+{
+ EXPECT_EQ(variant->error, test_secbits_set(0));
+}
+
+#define CHILD(...) \
+ do { \
+ pid_t child = vfork(); \
+ EXPECT_LE(0, child); \
+ if (child == 0) { \
+ __VA_ARGS__; \
+ _exit(0); \
+ } \
+ } while (0)
+
+TEST_F(secbits, exec)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits &= ~(SECBIT_EXEC_RESTRICT_FILE | SECBIT_EXEC_DENY_INTERACTIVE);
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+}
+
+TEST_F(secbits, check_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, check_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/exec/config b/tools/testing/selftests/exec/config
new file mode 100644
index 000000000000..c308079867b3
--- /dev/null
+++ b/tools/testing/selftests/exec/config
@@ -0,0 +1,2 @@
+CONFIG_BLK_DEV=y
+CONFIG_BLK_DEV_LOOP=y
diff --git a/tools/testing/selftests/exec/false.c b/tools/testing/selftests/exec/false.c
new file mode 100644
index 000000000000..104383ec3a79
--- /dev/null
+++ b/tools/testing/selftests/exec/false.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+int main(void)
+{
+ return 1;
+}
--
2.47.1
^ permalink raw reply related
* [PATCH v23 4/8] selftests/landlock: Add tests for execveat + AT_EXECVE_CHECK
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Günther Noack
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
Extend layout1.execute with the new AT_EXECVE_CHECK flag. The semantic
with AT_EXECVE_CHECK is the same as with a simple execve(2),
LANDLOCK_ACCESS_FS_EXECUTE is enforced the same way.
Cc: Günther Noack <gnoack@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-5-mic@digikod.net
---
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
---
tools/testing/selftests/landlock/fs_test.c | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 6788762188fe..cd66901be612 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -37,6 +37,10 @@
#include <linux/fs.h>
#include <linux/mount.h>
+/* Defines AT_EXECVE_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
#include "common.h"
#ifndef renameat2
@@ -2008,6 +2012,22 @@ static void test_execute(struct __test_metadata *const _metadata, const int err,
};
}
+static void test_check_exec(struct __test_metadata *const _metadata,
+ const int err, const char *const path)
+{
+ int ret;
+ char *const argv[] = { (char *)path, NULL };
+
+ ret = execveat(AT_FDCWD, path, argv, NULL,
+ AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ if (err) {
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(errno, err);
+ } else {
+ EXPECT_EQ(0, ret);
+ }
+}
+
TEST_F_FORK(layout1, execute)
{
const struct rule rules[] = {
@@ -2025,20 +2045,27 @@ TEST_F_FORK(layout1, execute)
copy_binary(_metadata, file1_s1d2);
copy_binary(_metadata, file1_s1d3);
+ /* Checks before file1_s1d1 being denied. */
+ test_execute(_metadata, 0, file1_s1d1);
+ test_check_exec(_metadata, 0, file1_s1d1);
+
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
ASSERT_EQ(0, test_open(dir_s1d1, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d1, O_RDONLY));
test_execute(_metadata, EACCES, file1_s1d1);
+ test_check_exec(_metadata, EACCES, file1_s1d1);
ASSERT_EQ(0, test_open(dir_s1d2, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
test_execute(_metadata, 0, file1_s1d2);
+ test_check_exec(_metadata, 0, file1_s1d2);
ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
test_execute(_metadata, 0, file1_s1d3);
+ test_check_exec(_metadata, 0, file1_s1d3);
}
TEST_F_FORK(layout1, link)
--
2.47.1
^ permalink raw reply related
* [PATCH v23 6/8] selftests: ktap_helpers: Fix uninitialized variable
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Kees Cook, Nícolas F. R. A. Prado
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
__ktap_test() may be called without the optional third argument which is
an issue for scripts using `set -u` to detect uninitialized variables
and potential bugs.
Fix this optional "directive" argument by either using the third
argument or an empty string.
This is required for the next commit to properly test script execution
control.
Cc: Kees Cook <kees@kernel.org>
Cc: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Fixes: 14571ab1ad21 ("kselftest: Add new test for detecting unprobed Devicetree devices")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-7-mic@digikod.net
---
Also sent as a standalone patch:
https://lore.kernel.org/r/20241127160342.31472-1-mic@digikod.net
---
tools/testing/selftests/kselftest/ktap_helpers.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh
index 79a125eb24c2..14e7f3ec3f84 100644
--- a/tools/testing/selftests/kselftest/ktap_helpers.sh
+++ b/tools/testing/selftests/kselftest/ktap_helpers.sh
@@ -40,7 +40,7 @@ ktap_skip_all() {
__ktap_test() {
result="$1"
description="$2"
- directive="$3" # optional
+ directive="${3:-}" # optional
local directive_str=
[ ! -z "$directive" ] && directive_str="# $directive"
--
2.47.1
^ permalink raw reply related
* [PATCH v23 5/8] samples/check-exec: Add set-exec
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
Add a simple tool to set SECBIT_EXEC_RESTRICT_FILE or
SECBIT_EXEC_DENY_INTERACTIVE before executing a command. This is useful
to easily test against enlighten script interpreters.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-6-mic@digikod.net
---
Changes since v19:
* Rename file and directory.
* Update securebits and related arguments.
* Remove useless call to prctl() when securebits are unchanged.
---
samples/Kconfig | 7 +++
samples/Makefile | 1 +
samples/check-exec/.gitignore | 1 +
samples/check-exec/Makefile | 14 ++++++
samples/check-exec/set-exec.c | 85 +++++++++++++++++++++++++++++++++++
5 files changed, 108 insertions(+)
create mode 100644 samples/check-exec/.gitignore
create mode 100644 samples/check-exec/Makefile
create mode 100644 samples/check-exec/set-exec.c
diff --git a/samples/Kconfig b/samples/Kconfig
index b288d9991d27..efa28ceadc42 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -291,6 +291,13 @@ config SAMPLE_CGROUP
help
Build samples that demonstrate the usage of the cgroup API.
+config SAMPLE_CHECK_EXEC
+ bool "Exec secure bits examples"
+ depends on CC_CAN_LINK && HEADERS_INSTALL
+ help
+ Build a tool to easily configure SECBIT_EXEC_RESTRICT_FILE and
+ SECBIT_EXEC_DENY_INTERACTIVE.
+
source "samples/rust/Kconfig"
endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index b85fa64390c5..f988202f3a30 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -3,6 +3,7 @@
subdir-$(CONFIG_SAMPLE_AUXDISPLAY) += auxdisplay
subdir-$(CONFIG_SAMPLE_ANDROID_BINDERFS) += binderfs
+subdir-$(CONFIG_SAMPLE_CHECK_EXEC) += check-exec
subdir-$(CONFIG_SAMPLE_CGROUP) += cgroup
obj-$(CONFIG_SAMPLE_CONFIGFS) += configfs/
obj-$(CONFIG_SAMPLE_CONNECTOR) += connector/
diff --git a/samples/check-exec/.gitignore b/samples/check-exec/.gitignore
new file mode 100644
index 000000000000..3f8119112ccf
--- /dev/null
+++ b/samples/check-exec/.gitignore
@@ -0,0 +1 @@
+/set-exec
diff --git a/samples/check-exec/Makefile b/samples/check-exec/Makefile
new file mode 100644
index 000000000000..d9f976e3ff98
--- /dev/null
+++ b/samples/check-exec/Makefile
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+userprogs-always-y := \
+ set-exec
+
+userccflags += -I usr/include
+
+.PHONY: all clean
+
+all:
+ $(MAKE) -C ../.. samples/check-exec/
+
+clean:
+ $(MAKE) -C ../.. M=samples/check-exec/ clean
diff --git a/samples/check-exec/set-exec.c b/samples/check-exec/set-exec.c
new file mode 100644
index 000000000000..ba86a60a20dd
--- /dev/null
+++ b/samples/check-exec/set-exec.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Simple tool to set SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE,
+ * before executing a command.
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+#include <errno.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+static void print_usage(const char *argv0)
+{
+ fprintf(stderr, "usage: %s -f|-i -- <cmd> [args]...\n\n", argv0);
+ fprintf(stderr, "Execute a command with\n");
+ fprintf(stderr, "- SECBIT_EXEC_RESTRICT_FILE set: -f\n");
+ fprintf(stderr, "- SECBIT_EXEC_DENY_INTERACTIVE set: -i\n");
+}
+
+int main(const int argc, char *const argv[], char *const *const envp)
+{
+ const char *cmd_path;
+ char *const *cmd_argv;
+ int opt, secbits_cur, secbits_new;
+ bool has_policy = false;
+
+ secbits_cur = prctl(PR_GET_SECUREBITS);
+ if (secbits_cur == -1) {
+ /*
+ * This should never happen, except with a buggy seccomp
+ * filter.
+ */
+ perror("ERROR: Failed to get securebits");
+ return 1;
+ }
+
+ secbits_new = secbits_cur;
+ while ((opt = getopt(argc, argv, "fi")) != -1) {
+ switch (opt) {
+ case 'f':
+ secbits_new |= SECBIT_EXEC_RESTRICT_FILE |
+ SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ has_policy = true;
+ break;
+ case 'i':
+ secbits_new |= SECBIT_EXEC_DENY_INTERACTIVE |
+ SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ has_policy = true;
+ break;
+ default:
+ print_usage(argv[0]);
+ return 1;
+ }
+ }
+
+ if (!argv[optind] || !has_policy) {
+ print_usage(argv[0]);
+ return 1;
+ }
+
+ if (secbits_cur != secbits_new &&
+ prctl(PR_SET_SECUREBITS, secbits_new)) {
+ perror("Failed to set secure bit(s).");
+ fprintf(stderr,
+ "Hint: The running kernel may not support this feature.\n");
+ return 1;
+ }
+
+ cmd_path = argv[optind];
+ cmd_argv = argv + optind;
+ fprintf(stderr, "Executing command...\n");
+ execvpe(cmd_path, cmd_argv, envp);
+ fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
+ strerror(errno));
+ return 1;
+}
--
2.47.1
^ permalink raw reply related
* [PATCH v23 7/8] samples/check-exec: Add an enlighten "inc" interpreter and 28 tests
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
Add a very simple script interpreter called "inc" that can evaluate two
different commands (one per line):
- "?" to initialize a counter from user's input;
- "+" to increment the counter (which is set to 0 by default).
It is enlighten to only interpret executable files according to
AT_EXECVE_CHECK and the related securebits:
# Executing a script with RESTRICT_FILE is only allowed if the script
# is executable:
./set-exec -f -- ./inc script-exec.inc # Allowed
./set-exec -f -- ./inc script-noexec.inc # Denied
# Executing stdin with DENY_INTERACTIVE is only allowed if stdin is an
# executable regular file:
./set-exec -i -- ./inc -i < script-exec.inc # Allowed
./set-exec -i -- ./inc -i < script-noexec.inc # Denied
# However, a pipe is not executable and it is then denied:
cat script-noexec.inc | ./set-exec -i -- ./inc -i # Denied
# Executing raw data (e.g. command argument) with DENY_INTERACTIVE is
# always denied.
./set-exec -i -- ./inc -c "+" # Denied
./inc -c "$(<script-ask.inc)" # Allowed
# To directly execute a script, we can update $PATH (used by `env`):
PATH="${PATH}:." ./script-exec.inc
# To execute several commands passed as argument:
Add a complete test suite to check the script interpreter against all
possible execution cases:
make TARGETS=exec kselftest-install
./tools/testing/selftests/kselftest_install/run_kselftest.sh
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241212174223.389435-8-mic@digikod.net
---
Changes since v21:
* Move the ktap_helpers.sh fix to a standalone previous commit (that
should be backported).
* Add missing SPDX-License-Identifier headers to test scripts.
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
Changes since v19:
* New patch.
---
samples/Kconfig | 4 +-
samples/check-exec/.gitignore | 1 +
samples/check-exec/Makefile | 1 +
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 +
tools/testing/selftests/exec/.gitignore | 2 +
tools/testing/selftests/exec/Makefile | 14 +-
.../selftests/exec/check-exec-tests.sh | 205 ++++++++++++++++++
11 files changed, 451 insertions(+), 3 deletions(-)
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 100755 tools/testing/selftests/exec/check-exec-tests.sh
diff --git a/samples/Kconfig b/samples/Kconfig
index efa28ceadc42..84a9d4e8d947 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -296,7 +296,9 @@ config SAMPLE_CHECK_EXEC
depends on CC_CAN_LINK && HEADERS_INSTALL
help
Build a tool to easily configure SECBIT_EXEC_RESTRICT_FILE and
- SECBIT_EXEC_DENY_INTERACTIVE.
+ SECBIT_EXEC_DENY_INTERACTIVE, and a simple script interpreter to
+ demonstrate how they should be used with execveat(2) +
+ AT_EXECVE_CHECK.
source "samples/rust/Kconfig"
diff --git a/samples/check-exec/.gitignore b/samples/check-exec/.gitignore
index 3f8119112ccf..cd759a19dacd 100644
--- a/samples/check-exec/.gitignore
+++ b/samples/check-exec/.gitignore
@@ -1 +1,2 @@
+/inc
/set-exec
diff --git a/samples/check-exec/Makefile b/samples/check-exec/Makefile
index d9f976e3ff98..c4f08ad0f8e3 100644
--- a/samples/check-exec/Makefile
+++ b/samples/check-exec/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
userprogs-always-y := \
+ inc \
set-exec
userccflags += -I usr/include
diff --git a/samples/check-exec/inc.c b/samples/check-exec/inc.c
new file mode 100644
index 000000000000..94b87569d2a2
--- /dev/null
+++ b/samples/check-exec/inc.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Very simple script interpreter that can evaluate two different commands (one
+ * per line):
+ * - "?" to initialize a counter from user's input;
+ * - "+" to increment the counter (which is set to 0 by default).
+ *
+ * See tools/testing/selftests/exec/check-exec-tests.sh and
+ * Documentation/userspace-api/check_exec.rst
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <linux/fcntl.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+/* Returns 1 on error, 0 otherwise. */
+static int interpret_buffer(char *buffer, size_t buffer_size)
+{
+ char *line, *saveptr = NULL;
+ long long number = 0;
+
+ /* Each command is the first character of a line. */
+ saveptr = NULL;
+ line = strtok_r(buffer, "\n", &saveptr);
+ while (line) {
+ if (*line != '#' && strlen(line) != 1) {
+ fprintf(stderr, "# ERROR: Unknown string\n");
+ return 1;
+ }
+ switch (*line) {
+ case '#':
+ /* Skips shebang and comments. */
+ break;
+ case '+':
+ /* Increments and prints the number. */
+ number++;
+ printf("%lld\n", number);
+ break;
+ case '?':
+ /* Reads integer from stdin. */
+ fprintf(stderr, "> Enter new number: \n");
+ if (scanf("%lld", &number) != 1) {
+ fprintf(stderr,
+ "# WARNING: Failed to read number from stdin\n");
+ }
+ break;
+ default:
+ fprintf(stderr, "# ERROR: Unknown character '%c'\n",
+ *line);
+ return 1;
+ }
+ line = strtok_r(NULL, "\n", &saveptr);
+ }
+ return 0;
+}
+
+/* Returns 1 on error, 0 otherwise. */
+static int interpret_stream(FILE *script, char *const script_name,
+ char *const *const envp, const bool restrict_stream)
+{
+ int err;
+ char *const script_argv[] = { script_name, NULL };
+ char buf[128] = {};
+ size_t buf_size = sizeof(buf);
+
+ /*
+ * We pass a valid argv and envp to the kernel to emulate a native
+ * script execution. We must use the script file descriptor instead of
+ * the script path name to avoid race conditions.
+ */
+ err = execveat(fileno(script), "", script_argv, envp,
+ AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ if (err && restrict_stream) {
+ perror("ERROR: Script execution check");
+ return 1;
+ }
+
+ /* Reads script. */
+ buf_size = fread(buf, 1, buf_size - 1, script);
+ return interpret_buffer(buf, buf_size);
+}
+
+static void print_usage(const char *argv0)
+{
+ fprintf(stderr, "usage: %s <script.inc> | -i | -c <command>\n\n",
+ argv0);
+ fprintf(stderr, "Example:\n");
+ fprintf(stderr, " ./set-exec -fi -- ./inc -i < script-exec.inc\n");
+}
+
+int main(const int argc, char *const argv[], char *const *const envp)
+{
+ int opt;
+ char *cmd = NULL;
+ char *script_name = NULL;
+ bool interpret_stdin = false;
+ FILE *script_file = NULL;
+ int secbits;
+ bool deny_interactive, restrict_file;
+ size_t arg_nb;
+
+ secbits = prctl(PR_GET_SECUREBITS);
+ if (secbits == -1) {
+ /*
+ * This should never happen, except with a buggy seccomp
+ * filter.
+ */
+ perror("ERROR: Failed to get securebits");
+ return 1;
+ }
+
+ deny_interactive = !!(secbits & SECBIT_EXEC_DENY_INTERACTIVE);
+ restrict_file = !!(secbits & SECBIT_EXEC_RESTRICT_FILE);
+
+ while ((opt = getopt(argc, argv, "c:i")) != -1) {
+ switch (opt) {
+ case 'c':
+ if (cmd) {
+ fprintf(stderr, "ERROR: Command already set");
+ return 1;
+ }
+ cmd = optarg;
+ break;
+ case 'i':
+ interpret_stdin = true;
+ break;
+ default:
+ print_usage(argv[0]);
+ return 1;
+ }
+ }
+
+ /* Checks that only one argument is used, or read stdin. */
+ arg_nb = !!cmd + !!interpret_stdin;
+ if (arg_nb == 0 && argc == 2) {
+ script_name = argv[1];
+ } else if (arg_nb != 1) {
+ print_usage(argv[0]);
+ return 1;
+ }
+
+ if (cmd) {
+ /*
+ * Other kind of interactive interpretations should be denied
+ * as well (e.g. CLI arguments passing script snippets,
+ * environment variables interpreted as script). However, any
+ * way to pass script files should only be restricted according
+ * to restrict_file.
+ */
+ if (deny_interactive) {
+ fprintf(stderr,
+ "ERROR: Interactive interpretation denied.\n");
+ return 1;
+ }
+
+ return interpret_buffer(cmd, strlen(cmd));
+ }
+
+ if (interpret_stdin && !script_name) {
+ script_file = stdin;
+ /*
+ * As for any execve(2) call, this path may be logged by the
+ * kernel.
+ */
+ script_name = "/proc/self/fd/0";
+ /*
+ * When stdin is used, it can point to a regular file or a
+ * pipe. Restrict stdin execution according to
+ * SECBIT_EXEC_DENY_INTERACTIVE but always allow executable
+ * files (which are not considered as interactive inputs).
+ */
+ return interpret_stream(script_file, script_name, envp,
+ deny_interactive);
+ } else if (script_name && !interpret_stdin) {
+ /*
+ * In this sample, we don't pass any argument to scripts, but
+ * otherwise we would have to forge an argv with such
+ * arguments.
+ */
+ script_file = fopen(script_name, "r");
+ if (!script_file) {
+ perror("ERROR: Failed to open script");
+ return 1;
+ }
+ /*
+ * Restricts file execution according to
+ * SECBIT_EXEC_RESTRICT_FILE.
+ */
+ return interpret_stream(script_file, script_name, envp,
+ restrict_file);
+ }
+
+ print_usage(argv[0]);
+ return 1;
+}
diff --git a/samples/check-exec/run-script-ask.inc b/samples/check-exec/run-script-ask.inc
new file mode 100755
index 000000000000..8ef0fdc37266
--- /dev/null
+++ b/samples/check-exec/run-script-ask.inc
@@ -0,0 +1,9 @@
+#!/usr/bin/env sh
+# SPDX-License-Identifier: BSD-3-Clause
+
+DIR="$(dirname -- "$0")"
+
+PATH="${PATH}:${DIR}"
+
+set -x
+"${DIR}/script-ask.inc"
diff --git a/samples/check-exec/script-ask.inc b/samples/check-exec/script-ask.inc
new file mode 100755
index 000000000000..720a8e649225
--- /dev/null
+++ b/samples/check-exec/script-ask.inc
@@ -0,0 +1,5 @@
+#!/usr/bin/env inc
+# SPDX-License-Identifier: BSD-3-Clause
+
+?
++
diff --git a/samples/check-exec/script-exec.inc b/samples/check-exec/script-exec.inc
new file mode 100755
index 000000000000..3245cb9d8dd1
--- /dev/null
+++ b/samples/check-exec/script-exec.inc
@@ -0,0 +1,4 @@
+#!/usr/bin/env inc
+# SPDX-License-Identifier: BSD-3-Clause
+
++
diff --git a/samples/check-exec/script-noexec.inc b/samples/check-exec/script-noexec.inc
new file mode 100644
index 000000000000..3245cb9d8dd1
--- /dev/null
+++ b/samples/check-exec/script-noexec.inc
@@ -0,0 +1,4 @@
+#!/usr/bin/env inc
+# SPDX-License-Identifier: BSD-3-Clause
+
++
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index a32c63bb4df1..7f3d1ae762ec 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -11,9 +11,11 @@ non-regular
null-argv
/check-exec
/false
+/inc
/load_address.*
!load_address.c
/recursion-depth
+/set-exec
xxxxxxxx*
pipe
S_I*.test
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 8713d1c862ae..45a3cfc435cf 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -10,9 +10,9 @@ ALIGN_PIES := $(patsubst %,load_address.%,$(ALIGNS))
ALIGN_STATIC_PIES := $(patsubst %,load_address.static.%,$(ALIGNS))
ALIGNMENT_TESTS := $(ALIGN_PIES) $(ALIGN_STATIC_PIES)
-TEST_PROGS := binfmt_script.py
+TEST_PROGS := binfmt_script.py check-exec-tests.sh
TEST_GEN_PROGS := execveat non-regular $(ALIGNMENT_TESTS)
-TEST_GEN_PROGS_EXTENDED := false
+TEST_GEN_PROGS_EXTENDED := false inc set-exec script-exec.inc script-noexec.inc
TEST_GEN_FILES := execveat.symlink execveat.denatured script subdir
# Makefile is a run-time dependency, since it's accessed by the execveat test
TEST_FILES := Makefile
@@ -26,6 +26,8 @@ EXTRA_CLEAN := $(OUTPUT)/subdir.moved $(OUTPUT)/execveat.moved $(OUTPUT)/xxxxx*
include ../lib.mk
+CHECK_EXEC_SAMPLES := $(top_srcdir)/samples/check-exec
+
$(OUTPUT)/subdir:
mkdir -p $@
$(OUTPUT)/script: Makefile
@@ -45,3 +47,11 @@ $(OUTPUT)/load_address.static.0x%: load_address.c
-fPIE -static-pie $< -o $@
$(OUTPUT)/false: false.c
$(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
+$(OUTPUT)/inc: $(CHECK_EXEC_SAMPLES)/inc.c
+ $(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+$(OUTPUT)/set-exec: $(CHECK_EXEC_SAMPLES)/set-exec.c
+ $(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+$(OUTPUT)/script-exec.inc: $(CHECK_EXEC_SAMPLES)/script-exec.inc
+ cp $< $@
+$(OUTPUT)/script-noexec.inc: $(CHECK_EXEC_SAMPLES)/script-noexec.inc
+ cp $< $@
diff --git a/tools/testing/selftests/exec/check-exec-tests.sh b/tools/testing/selftests/exec/check-exec-tests.sh
new file mode 100755
index 000000000000..87102906ae3c
--- /dev/null
+++ b/tools/testing/selftests/exec/check-exec-tests.sh
@@ -0,0 +1,205 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test the "inc" interpreter.
+#
+# See include/uapi/linux/securebits.h, include/uapi/linux/fcntl.h and
+# samples/check-exec/inc.c
+#
+# Copyright © 2024 Microsoft Corporation
+
+set -u -e -o pipefail
+
+EXPECTED_OUTPUT="1"
+exec 2>/dev/null
+
+DIR="$(dirname $(readlink -f "$0"))"
+source "${DIR}"/../kselftest/ktap_helpers.sh
+
+exec_direct() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Updates PATH for `env` to execute the `inc` interpreter.
+ out="$(PATH="." "$@" "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for direct file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for direct file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_indirect() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Script passed as argument.
+ out="$("$@" ./inc "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for indirect file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for indirect file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_stdin_reg() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Executing stdin must be allowed if the related file is executable.
+ out="$("$@" ./inc -i < "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for stdin regular file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for stdin regular file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_stdin_pipe() {
+ local expect="$1"
+ shift
+ local ret=0
+ local out
+
+ # A pipe is not executable.
+ out="$(cat script-exec.inc | "$@" ./inc -i)" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for stdin pipe execution: ${ret}"
+ return 1
+ fi
+}
+
+exec_argument() {
+ local expect="$1"
+ local ret=0
+ shift
+ local out
+
+ # Script not coming from a file must not be executed.
+ out="$("$@" ./inc -c "$(< script-exec.inc)")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for arbitrary argument execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for arbitrary argument execution: ${out}"
+ return 1
+ fi
+}
+
+exec_interactive() {
+ exec_stdin_pipe "$@"
+ exec_argument "$@"
+}
+
+ktap_test() {
+ ktap_test_result "$*" "$@"
+}
+
+ktap_print_header
+ktap_set_plan 28
+
+# Without secbit configuration, nothing is changed.
+
+ktap_print_msg "By default, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc
+ktap_test exec_indirect 0 script-exec.inc
+
+ktap_print_msg "By default, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc
+
+ktap_print_msg "By default, non-executable scripts are allowed to be interpreted, but not directly executed."
+# We get 126 because of direct execution by Bash.
+ktap_test exec_direct 126 script-noexec.inc
+ktap_test exec_indirect 0 script-noexec.inc
+
+ktap_print_msg "By default, non-executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-noexec.inc
+
+ktap_print_msg "By default, interactive commands are allowed to be interpreted."
+ktap_test exec_interactive 0
+
+# With only file restriction: protect non-malicious users from inadvertent errors (e.g. python ~/Downloads/*.py).
+
+ktap_print_msg "With -f, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -f --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, non-executable scripts are not allowed to be executed nor interpreted."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -f --
+ktap_test exec_indirect 1 script-noexec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, non-executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-noexec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, interactive commands are allowed to be interpreted."
+ktap_test exec_interactive 0 ./set-exec -f --
+
+# With only denied interactive commands: check or monitor script content (e.g. with LSM).
+
+ktap_print_msg "With -i, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -i --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, non-executable scripts are allowed to be interpreted, but not directly executed."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -i --
+ktap_test exec_indirect 0 script-noexec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, non-executable stdin is not allowed to be interpreted."
+ktap_test exec_stdin_reg 1 script-noexec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, interactive commands are not allowed to be interpreted."
+ktap_test exec_interactive 1 ./set-exec -i --
+
+# With both file restriction and denied interactive commands: only allow executable scripts.
+
+ktap_print_msg "With -fi, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -fi --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, non-executable scripts are not allowed to be interpreted nor executed."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -fi --
+ktap_test exec_indirect 1 script-noexec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, non-executable stdin is not allowed to be interpreted."
+ktap_test exec_stdin_reg 1 script-noexec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, interactive commands are not allowed to be interpreted."
+ktap_test exec_interactive 1 ./set-exec -fi --
+
+ktap_finished
--
2.47.1
^ permalink raw reply related
* [PATCH v23 8/8] ima: instantiate the bprm_creds_for_exec() hook
From: Mickaël Salaün @ 2024-12-12 17:42 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, 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, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Stefan Berger
In-Reply-To: <20241212174223.389435-1-mic@digikod.net>
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>
Tested-by: Stefan Berger <stefanb@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/20241212174223.389435-9-mic@digikod.net
---
I added both a Reviewed-by and a Signed-off-by because I may not be the
committer.
Changes since v22:
* Add Stefan's Tested-by.
* Replace AUDIT_INTEGRITY_DATA_CHECK with AUDIT_INTEGRITY_USERSPACE as
spotted by Paul and suggested by Mimi.
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..d9a069b4a775 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_USERSPACE 1808 /* Userspace enforced data integrity */
#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 884a3533f7af..f435eff4667f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/file.h>
+#include <linux/binfmts.h>
#include <linux/fs.h>
#include <linux/xattr.h>
#include <linux/magic.h>
@@ -469,6 +470,17 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
return rc;
}
+static bool is_bprm_creds_for_exec(enum ima_hooks func, struct file *file)
+{
+ struct linux_binprm *bprm;
+
+ if (func == BPRM_CHECK) {
+ bprm = container_of(&file, struct linux_binprm, file);
+ return bprm->is_check;
+ }
+ return false;
+}
+
/*
* ima_appraise_measurement - appraise file measurement
*
@@ -483,6 +495,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
int xattr_len, const struct modsig *modsig)
{
static const char op[] = "appraise_data";
+ int audit_msgno = AUDIT_INTEGRITY_DATA;
const char *cause = "unknown";
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
@@ -494,6 +507,16 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
return INTEGRITY_UNKNOWN;
+ /*
+ * Unlike any of the other LSM hooks where the kernel enforces file
+ * integrity, enforcing file integrity for the bprm_creds_for_exec()
+ * LSM hook with the AT_EXECVE_CHECK flag is left up to the discretion
+ * of the script interpreter(userspace). Differentiate kernel and
+ * userspace enforced integrity audit messages.
+ */
+ if (is_bprm_creds_for_exec(func, file))
+ audit_msgno = AUDIT_INTEGRITY_USERSPACE;
+
/* If reading the xattr failed and there's no modsig, error out. */
if (rc <= 0 && !try_modsig) {
if (rc && rc != -ENODATA)
@@ -569,7 +592,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
(iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
status = INTEGRITY_FAIL;
cause = "unverifiable-signature";
- integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
+ integrity_audit_msg(audit_msgno, inode, filename,
op, cause, rc, 0);
} else if (status != INTEGRITY_PASS) {
/* Fix mode, but don't replace file signatures. */
@@ -589,7 +612,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
status = INTEGRITY_PASS;
}
- integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
+ integrity_audit_msg(audit_msgno, inode, filename,
op, cause, rc, 0);
} else {
ima_cache_flags(iint, func);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 9b87556b03a7..9f9897a7c217 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -554,6 +554,34 @@ static int ima_bprm_check(struct linux_binprm *bprm)
MAY_EXEC, CREDS_CHECK);
}
+/**
+ * ima_bprm_creds_for_exec - collect/store/appraise measurement.
+ * @bprm: contains the linux_binprm structure
+ *
+ * Based on the IMA policy and the execveat(2) AT_EXECVE_CHECK flag, measure
+ * and appraise the integrity of a file to be executed by script interpreters.
+ * Unlike any of the other LSM hooks where the kernel enforces file integrity,
+ * enforcing file integrity is left up to the discretion of the script
+ * interpreter (userspace).
+ *
+ * On success return 0. On integrity appraisal error, assuming the file
+ * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
+ */
+static int ima_bprm_creds_for_exec(struct linux_binprm *bprm)
+{
+ /*
+ * As security_bprm_check() is called multiple times, both
+ * the script and the shebang interpreter are measured, appraised,
+ * and audited. Limit usage of this LSM hook to just measuring,
+ * appraising, and auditing the indirect script execution
+ * (e.g. ./sh example.sh).
+ */
+ if (!bprm->is_check)
+ return 0;
+
+ return ima_bprm_check(bprm);
+}
+
/**
* ima_file_check - based on policy, collect/store measurement.
* @file: pointer to the file to be measured
@@ -1174,6 +1202,7 @@ static int __init init_ima(void)
static struct security_hook_list ima_hooks[] __ro_after_init = {
LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
+ LSM_HOOK_INIT(bprm_creds_for_exec, ima_bprm_creds_for_exec),
LSM_HOOK_INIT(file_post_open, ima_file_check),
LSM_HOOK_INIT(inode_post_create_tmpfile, ima_post_create_tmpfile),
LSM_HOOK_INIT(file_release, ima_file_free),
--
2.47.1
^ permalink raw reply related
* I urge you to understand my viewpoint accurately.
From: TCWM178536 @ 2024-12-13 4:24 UTC (permalink / raw)
To: linux-api
I am Tomasz Chmielewski, a Portfolio Manager and Chartered
Financial Analyst affiliated with Iwoca Poland Sp. Z OO in
Poland. I have the privilege of working with distinguished
investors who are eager to support your company's current
initiatives, thereby broadening their investment portfolios. If
this proposal aligns with your interests, I invite you to
respond, and I will gladly share more information to assist you.
Yours sincerely,
Tomasz Chmielewski Warsaw, Mazowieckie,
Poland.
^ permalink raw reply
* [PATCH v7 00/30] NT synchronization primitive driver
From: Elizabeth Figura @ 2024-12-13 19:34 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/ntsync7
== Previous versions ==
Changes from v6:
* 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, which simplifies the API a bit.
* Link to v6: https://lore.kernel.org/lkml/20241209185904.507350-1-zfigura@codeweavers.com/
* 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 (30):
ntsync: Return the fd from NTSYNC_IOC_CREATE_SEM.
ntsync: Rename NTSYNC_IOC_SEM_POST to NTSYNC_IOC_SEM_RELEASE.
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 | 385 +++++
MAINTAINERS | 9 +
drivers/misc/Kconfig | 1 -
drivers/misc/ntsync.c | 992 +++++++++++-
include/uapi/linux/ntsync.h | 42 +-
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 | 1343 +++++++++++++++++
11 files changed, 2767 insertions(+), 16 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 v7 01/30] ntsync: Return the fd from NTSYNC_IOC_CREATE_SEM.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-1-zfigura@codeweavers.com>
Simplify the user API a bit by returning the fd as return value from the ioctl
instead of through the argument pointer.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 7 ++-----
include/uapi/linux/ntsync.h | 3 +--
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 4954553b7baa..2e7f698268c1 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -165,7 +165,6 @@ static int ntsync_obj_get_fd(struct ntsync_obj *obj)
static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
{
- struct ntsync_sem_args __user *user_args = argp;
struct ntsync_sem_args args;
struct ntsync_obj *sem;
int fd;
@@ -182,12 +181,10 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
sem->u.sem.count = args.count;
sem->u.sem.max = args.max;
fd = ntsync_obj_get_fd(sem);
- if (fd < 0) {
+ if (fd < 0)
kfree(sem);
- return fd;
- }
- return put_user(fd, &user_args->sem);
+ return fd;
}
static int ntsync_char_open(struct inode *inode, struct file *file)
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index dcfa38fdc93c..27d8cb3dd5b7 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -11,12 +11,11 @@
#include <linux/types.h>
struct ntsync_sem_args {
- __u32 sem;
__u32 count;
__u32 max;
};
-#define NTSYNC_IOC_CREATE_SEM _IOWR('N', 0x80, struct ntsync_sem_args)
+#define NTSYNC_IOC_CREATE_SEM _IOW ('N', 0x80, struct ntsync_sem_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v7 08/30] ntsync: Introduce NTSYNC_IOC_CREATE_EVENT.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 | 59 +++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 6 ++++
2 files changed, 65 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 03768ac25425..3e8827b6f480 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.
@@ -622,6 +655,27 @@ static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp)
return fd;
}
+static int ntsync_create_event(struct ntsync_device *dev, void __user *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;
+}
+
static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
{
struct file *file = fget(fd);
@@ -752,6 +806,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;
}
}
@@ -941,6 +998,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 633958d90be3..e08aa02f4dcc 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -20,6 +20,11 @@ struct ntsync_mutex_args {
__u32 count;
};
+struct ntsync_event_args {
+ __u32 manual;
+ __u32 signaled;
+};
+
#define NTSYNC_WAIT_REALTIME 0x1
struct ntsync_wait_args {
@@ -38,6 +43,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 _IOW ('N', 0x84, struct ntsync_mutex_args)
+#define NTSYNC_IOC_CREATE_EVENT _IOW ('N', 0x87, struct ntsync_event_args)
#define NTSYNC_IOC_SEM_RELEASE _IOWR('N', 0x81, __u32)
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
--
2.45.2
^ permalink raw reply related
* [PATCH v7 02/30] ntsync: Rename NTSYNC_IOC_SEM_POST to NTSYNC_IOC_SEM_RELEASE.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-1-zfigura@codeweavers.com>
Use the more common "release" terminology, which is also the term used by NT,
instead of "post" (which is used by POSIX).
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 10 +++++-----
include/uapi/linux/ntsync.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 2e7f698268c1..cb3a3bd97ba0 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -57,7 +57,7 @@ struct ntsync_device {
* Actually change the semaphore state, returning -EOVERFLOW if it is made
* invalid.
*/
-static int post_sem_state(struct ntsync_obj *sem, __u32 count)
+static int release_sem_state(struct ntsync_obj *sem, __u32 count)
{
__u32 sum;
@@ -71,7 +71,7 @@ static int post_sem_state(struct ntsync_obj *sem, __u32 count)
return 0;
}
-static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
+static int ntsync_sem_release(struct ntsync_obj *sem, void __user *argp)
{
__u32 __user *user_args = argp;
__u32 prev_count;
@@ -87,7 +87,7 @@ static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp)
spin_lock(&sem->lock);
prev_count = sem->u.sem.count;
- ret = post_sem_state(sem, args);
+ ret = release_sem_state(sem, args);
spin_unlock(&sem->lock);
@@ -114,8 +114,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
void __user *argp = (void __user *)parm;
switch (cmd) {
- case NTSYNC_IOC_SEM_POST:
- return ntsync_sem_post(obj, argp);
+ case NTSYNC_IOC_SEM_RELEASE:
+ return ntsync_sem_release(obj, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 27d8cb3dd5b7..9af9d8125553 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -17,6 +17,6 @@ struct ntsync_sem_args {
#define NTSYNC_IOC_CREATE_SEM _IOW ('N', 0x80, struct ntsync_sem_args)
-#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
+#define NTSYNC_IOC_SEM_RELEASE _IOWR('N', 0x81, __u32)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v7 09/30] ntsync: Introduce NTSYNC_IOC_EVENT_SET.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 3e8827b6f480..0a87f8ad5993 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 e08aa02f4dcc..db2512f6e3f4 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -48,5 +48,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_SEM_RELEASE _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 v7 10/30] ntsync: Introduce NTSYNC_IOC_EVENT_RESET.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 0a87f8ad5993..b31443aa9692 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 db2512f6e3f4..d74c4e4f93d8 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -49,5 +49,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 v7 11/30] ntsync: Introduce NTSYNC_IOC_EVENT_PULSE.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 b31443aa9692..141ebe8a7d4f 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 d74c4e4f93d8..9eab7666b8b9 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -50,5 +50,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 v7 12/30] ntsync: Introduce NTSYNC_IOC_SEM_READ.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 | 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 141ebe8a7d4f..b2043412c5cd 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -583,6 +583,28 @@ 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;
+
+ 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 +624,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case NTSYNC_IOC_SEM_RELEASE:
return ntsync_sem_release(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 9eab7666b8b9..e36b4cfb7d34 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -51,5 +51,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 v7 04/30] ntsync: Introduce NTSYNC_IOC_WAIT_ALL.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 | 336 ++++++++++++++++++++++++++++++++++--
include/uapi/linux/ntsync.h | 1 +
2 files changed, 323 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 900cc5ce5761..c8beb5504bcc 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 release_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 release_sem_state(struct ntsync_obj *sem, __u32 count)
static int ntsync_sem_release(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_release(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)
+ ret = release_sem_state(sem, args);
+ 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;
}
@@ -301,7 +512,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;
@@ -324,6 +535,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++) {
@@ -333,6 +545,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;
@@ -362,13 +584,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;
@@ -378,9 +601,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 */
@@ -391,9 +614,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 */
@@ -406,9 +629,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);
}
@@ -430,6 +653,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;
@@ -438,6 +742,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);
@@ -461,6 +767,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 40ffdc41d5bb..d64420ffbcd7 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -30,6 +30,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_CREATE_SEM _IOW ('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_RELEASE _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v7 03/30] ntsync: Introduce NTSYNC_IOC_WAIT_ANY.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 | 247 +++++++++++++++++++++++++++++++++++-
include/uapi/linux/ntsync.h | 14 ++
2 files changed, 260 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index cb3a3bd97ba0..900cc5ce5761 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.
@@ -87,7 +137,9 @@ static int ntsync_sem_release(struct ntsync_obj *sem, void __user *argp)
spin_lock(&sem->lock);
prev_count = sem->u.sem.count;
- ret = release_sem_state(sem, args);
+ 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;
}
@@ -187,6 +240,196 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
return fd;
}
+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;
@@ -218,6 +461,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 9af9d8125553..40ffdc41d5bb 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -15,7 +15,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 _IOW ('N', 0x80, struct ntsync_sem_args)
+#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
#define NTSYNC_IOC_SEM_RELEASE _IOWR('N', 0x81, __u32)
--
2.45.2
^ permalink raw reply related
* [PATCH v7 05/30] ntsync: Introduce NTSYNC_IOC_CREATE_MUTEX.
From: Elizabeth Figura @ 2024-12-13 19:34 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: <20241213193511.457338-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 | 74 +++++++++++++++++++++++++++++++++++--
include/uapi/linux/ntsync.h | 9 ++++-
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index c8beb5504bcc..a2826dbff2f4 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.
@@ -451,6 +489,30 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
return fd;
}
+static int ntsync_create_mutex(struct ntsync_device *dev, void __user *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;
+}
+
static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
{
struct file *file = fget(fd);
@@ -520,7 +582,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)
@@ -534,6 +596,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;
@@ -576,6 +639,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;
}
}
@@ -765,6 +831,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 d64420ffbcd7..bb7fb94f5856 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -15,6 +15,11 @@ struct ntsync_sem_args {
__u32 max;
};
+struct ntsync_mutex_args {
+ __u32 owner;
+ __u32 count;
+};
+
#define NTSYNC_WAIT_REALTIME 0x1
struct ntsync_wait_args {
@@ -23,7 +28,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
@@ -31,6 +37,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_CREATE_SEM _IOW ('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 _IOW ('N', 0x84, struct ntsync_mutex_args)
#define NTSYNC_IOC_SEM_RELEASE _IOWR('N', 0x81, __u32)
--
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