* [PATCH v7 30/30] ntsync: No longer depend on BROKEN.
From: Elizabeth Figura @ 2024-12-13 19:35 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>
f5b335dc025cfee90957efa90dc72fada0d5abb4 ("misc: ntsync: mark driver as "broken"
to prevent from building") was committed to avoid the driver being used while
only part of its functionality was released. Since the rest of the functionality
has now been committed, revert this.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 09cbe3f0ab1e..fb772bfe27c3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -517,7 +517,6 @@ config OPEN_DICE
config NTSYNC
tristate "NT synchronization primitive emulation"
- depends on BROKEN
help
This module provides kernel support for emulation of Windows NT
synchronization primitives. It is not a hardware driver.
--
2.45.2
^ permalink raw reply related
* [PATCH v3 0/3] futex: Create set_robust_list2
From: André Almeida @ 2024-12-17 17:49 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, Nathan Chancellor,
Vinicius Peixoto, André Almeida
This patch adds a new robust_list() syscall. The current syscall
can't be expanded to cover the following use case, so a new one is
needed. This new syscall allows users to set multiple robust lists per
process and to have either 32bit or 64bit pointers in the list.
* Use case
FEX-Emu[1] is an application that runs x86 and x86-64 binaries on an
AArch64 Linux host. One of the tasks of FEX-Emu is to translate syscalls
from one platform to another. Existing set_robust_list() can't be easily
translated because of two limitations:
1) x86 apps can have 32bit pointers robust lists. For a x86-64 kernel
this is not a problem, because of the compat entry point. But there's
no such compat entry point for AArch64, so the kernel would do the
pointer arithmetic wrongly. Is also unviable to userspace to keep
track every addition/removal to the robust list and keep a 64bit
version of it somewhere else to feed the kernel. Thus, the new
interface has an option of telling the kernel if the list is filled
with 32bit or 64bit pointers.
2) Apps can set just one robust list (in theory, x86-64 can set two if
they also use the compat entry point). That means that when a x86 app
asks FEX-Emu to call set_robust_list(), FEX have two options: to
overwrite their own robust list pointer and make the app robust, or
to ignore the app robust list and keep the emulator robust. The new
interface allows for multiple robust lists per application, solving
this.
* Interface
This is the proposed interface:
long set_robust_list2(void *head, int index, unsigned int flags)
`head` is the head of the userspace struct robust_list_head, just as old
set_robust_list(). It needs to be a void pointer since it can point to a normal
robust_list_head or a compat_robust_list_head.
`flags` can be used for defining the list type:
enum robust_list_type {
ROBUST_LIST_32BIT,
ROBUST_LIST_64BIT,
};
`index` is the index in the internal robust_list's linked list (the naming
starts to get confusing, I reckon). If `index == -1`, that means that user wants
to set a new robust_list, and the kernel will append it in the end of the list,
assign a new index and return this index to the user. If `index >= 0`, that
means that user wants to re-set `*head` of an already existing list (similarly
to what happens when you call set_robust_list() twice with different `*head`).
If `index` is out of range, or it points to a non-existing robust_list, or if
the internal list is full, an error is returned.
* Implementation
The implementation re-uses most of the existing robust list interface as
possible. The new task_struct member `struct list_head robust_list2` is just a
linked list where new lists are appended as the user requests more lists, and by
futex_cleanup(), the kernel walks through the internal list feeding
exit_robust_list() with the robust_list's.
This implementation supports up to 10 lists (defined at ROBUST_LISTS_PER_TASK),
but it was an arbitrary number for this RFC. For the described use case above, 4
should be enough, I'm not sure which should be the limit.
It doesn't support list removal (should it support?). It doesn't have a proper
get_robust_list2() yet as well, but I can add it in a next revision. We could
also have a generic robust_list() syscall that can be used to set/get and be
controlled by flags.
The new interface has a `unsigned int flags` argument, making it
extensible for future use cases as well.
* Testing
I will provide a selftest similar to the one I proposed for the current
interface here:
https://lore.kernel.org/lkml/20241010011142.905297-1-andrealmeid@igalia.com/
Also, FEX-Emu added support for this interface to validate it:
https://github.com/FEX-Emu/FEX/pull/3966
Feedback is very welcomed!
Thanks,
André
[1] https://github.com/FEX-Emu/FEX
Changelog:
- Old syscall set_robust_list() adds new head to the internal linked list of
robust lists pointers, instead of having a field just for them. Remove
tsk->robust_list and use only tsk->robust_list2
v2: https://lore.kernel.org/lkml/20241101162147.284993-1-andrealmeid@igalia.com/
- Added a patch to properly deal with exit_robust_list() in 64bit vs 32bit
- Wired-up syscall for all archs
- Added more of the cover letter to the commit message
v1: https://lore.kernel.org/lkml/20241024145735.162090-1-andrealmeid@igalia.com/
André Almeida (3):
futex: Use explicit sizes for compat_exit_robust_list
futex: Create set_robust_list2
futex: Wire up set_robust_list2 syscall
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
include/linux/compat.h | 12 +-
include/linux/futex.h | 16 ++-
include/linux/sched.h | 5 +-
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/futex.h | 24 ++++
kernel/futex/core.c | 130 ++++++++++++++++----
kernel/futex/futex.h | 5 +
kernel/futex/syscalls.c | 82 +++++++++++-
kernel/sys_ni.c | 1 +
scripts/syscall.tbl | 1 +
25 files changed, 249 insertions(+), 46 deletions(-)
--
2.47.1
^ permalink raw reply
* [PATCH v3 1/3] futex: Use explicit sizes for compat_exit_robust_list
From: André Almeida @ 2024-12-17 17:49 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, Nathan Chancellor,
Vinicius Peixoto, André Almeida
In-Reply-To: <20241217174958.477692-1-andrealmeid@igalia.com>
There are two functions for handling robust lists during the task
exit: exit_robust_list() and compat_exit_robust_list(). The first one
handles either 64bit or 32bit lists, depending if it's a 64bit or 32bit
kernel. The compat_exit_robust_list() only exists in 64bit kernels that
supports 32bit syscalls, and handles 32bit lists.
For the new syscall set_robust_list2(), 64bit kernels need to be able to
handle 32bit lists despite having or not support for 32bit syscalls, so
make compat_exit_robust_list() exist regardless of compat_ config.
Also, use explicitly sizing, otherwise in a 32bit kernel both
exit_robust_list() and compat_exit_robust_list() would be the exactly
same function, with none of them dealing with 64bit robust lists.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
include/linux/compat.h | 12 +----------
include/linux/futex.h | 11 +++++++++++
include/linux/sched.h | 2 +-
kernel/futex/core.c | 44 ++++++++++++++++++++++++++---------------
kernel/futex/syscalls.c | 4 ++--
5 files changed, 43 insertions(+), 30 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 56cebaff0c91..968a9135ff48 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -385,16 +385,6 @@ struct compat_ifconf {
compat_caddr_t ifcbuf;
};
-struct compat_robust_list {
- compat_uptr_t next;
-};
-
-struct compat_robust_list_head {
- struct compat_robust_list list;
- compat_long_t futex_offset;
- compat_uptr_t list_op_pending;
-};
-
#ifdef CONFIG_COMPAT_OLD_SIGACTION
struct compat_old_sigaction {
compat_uptr_t sa_handler;
@@ -672,7 +662,7 @@ asmlinkage long compat_sys_waitid(int, compat_pid_t,
struct compat_siginfo __user *, int,
struct compat_rusage __user *);
asmlinkage long
-compat_sys_set_robust_list(struct compat_robust_list_head __user *head,
+compat_sys_set_robust_list(struct robust_list_head32 __user *head,
compat_size_t len);
asmlinkage long
compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
diff --git a/include/linux/futex.h b/include/linux/futex.h
index b70df27d7e85..8217b5ebdd9c 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -53,6 +53,17 @@ union futex_key {
#define FUTEX_KEY_INIT (union futex_key) { .both = { .ptr = 0ULL } }
#ifdef CONFIG_FUTEX
+
+struct robust_list32 {
+ u32 next;
+};
+
+struct robust_list_head32 {
+ struct robust_list32 list;
+ s32 futex_offset;
+ u32 list_op_pending;
+};
+
enum {
FUTEX_STATE_OK,
FUTEX_STATE_EXITING,
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 66b311fbd5d6..da573c16e93e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1292,7 +1292,7 @@ struct task_struct {
#ifdef CONFIG_FUTEX
struct robust_list_head __user *robust_list;
#ifdef CONFIG_COMPAT
- struct compat_robust_list_head __user *compat_robust_list;
+ struct robust_list_head32 __user *compat_robust_list;
#endif
struct list_head pi_state_list;
struct futex_pi_state *pi_state_cache;
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index ebdd76b4ecbb..cc2d0f00cd6b 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -768,13 +768,14 @@ static inline int fetch_robust_entry(struct robust_list __user **entry,
return 0;
}
+#ifdef CONFIG_64BIT
/*
* Walk curr->robust_list (very carefully, it's a userspace list!)
* and mark any locks found there dead, and notify any waiters.
*
* We silently return on any sign of list-walking problem.
*/
-static void exit_robust_list(struct task_struct *curr)
+static void exit_robust_list64(struct task_struct *curr)
{
struct robust_list_head __user *head = curr->robust_list;
struct robust_list __user *entry, *next_entry, *pending;
@@ -835,8 +836,13 @@ static void exit_robust_list(struct task_struct *curr)
curr, pip, HANDLE_DEATH_PENDING);
}
}
+#else
+static void exit_robust_list64(struct task_struct *curr)
+{
+ pr_warn("32bit kernel should not allow ROBUST_LIST_64BIT");
+}
+#endif
-#ifdef CONFIG_COMPAT
static void __user *futex_uaddr(struct robust_list __user *entry,
compat_long_t futex_offset)
{
@@ -850,13 +856,13 @@ static void __user *futex_uaddr(struct robust_list __user *entry,
* Fetch a robust-list pointer. Bit 0 signals PI futexes:
*/
static inline int
-compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
- compat_uptr_t __user *head, unsigned int *pi)
+fetch_robust_entry32(u32 *uentry, struct robust_list __user **entry,
+ u32 __user *head, unsigned int *pi)
{
if (get_user(*uentry, head))
return -EFAULT;
- *entry = compat_ptr((*uentry) & ~1);
+ *entry = (void __user *)(unsigned long)((*uentry) & ~1);
*pi = (unsigned int)(*uentry) & 1;
return 0;
@@ -868,21 +874,21 @@ compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **ent
*
* We silently return on any sign of list-walking problem.
*/
-static void compat_exit_robust_list(struct task_struct *curr)
+static void exit_robust_list32(struct task_struct *curr)
{
- struct compat_robust_list_head __user *head = curr->compat_robust_list;
+ struct robust_list_head32 __user *head = curr->compat_robust_list;
struct robust_list __user *entry, *next_entry, *pending;
unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
unsigned int next_pi;
- compat_uptr_t uentry, next_uentry, upending;
- compat_long_t futex_offset;
+ u32 uentry, next_uentry, upending;
+ s32 futex_offset;
int rc;
/*
* Fetch the list head (which was registered earlier, via
* sys_set_robust_list()):
*/
- if (compat_fetch_robust_entry(&uentry, &entry, &head->list.next, &pi))
+ if (fetch_robust_entry32((u32 *)&uentry, &entry, (u32 *)&head->list.next, &pi))
return;
/*
* Fetch the relative futex offset:
@@ -893,7 +899,7 @@ static void compat_exit_robust_list(struct task_struct *curr)
* Fetch any possibly pending lock-add first, and handle it
* if it exists:
*/
- if (compat_fetch_robust_entry(&upending, &pending,
+ if (fetch_robust_entry32(&upending, &pending,
&head->list_op_pending, &pip))
return;
@@ -903,8 +909,8 @@ static void compat_exit_robust_list(struct task_struct *curr)
* Fetch the next entry in the list before calling
* handle_futex_death:
*/
- rc = compat_fetch_robust_entry(&next_uentry, &next_entry,
- (compat_uptr_t __user *)&entry->next, &next_pi);
+ rc = fetch_robust_entry32(&next_uentry, &next_entry,
+ (u32 __user *)&entry->next, &next_pi);
/*
* A pending lock might already be on the list, so
* dont process it twice:
@@ -935,7 +941,6 @@ static void compat_exit_robust_list(struct task_struct *curr)
handle_futex_death(uaddr, curr, pip, HANDLE_DEATH_PENDING);
}
}
-#endif
#ifdef CONFIG_FUTEX_PI
@@ -1018,14 +1023,21 @@ static inline void exit_pi_state_list(struct task_struct *curr) { }
static void futex_cleanup(struct task_struct *tsk)
{
+#ifdef CONFIG_64BIT
if (unlikely(tsk->robust_list)) {
- exit_robust_list(tsk);
+ exit_robust_list64(tsk);
tsk->robust_list = NULL;
}
+#else
+ if (unlikely(tsk->robust_list)) {
+ exit_robust_list32(tsk);
+ tsk->robust_list = NULL;
+ }
+#endif
#ifdef CONFIG_COMPAT
if (unlikely(tsk->compat_robust_list)) {
- compat_exit_robust_list(tsk);
+ exit_robust_list32(tsk);
tsk->compat_robust_list = NULL;
}
#endif
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 4b6da9116aa6..dba193dfd216 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -440,7 +440,7 @@ SYSCALL_DEFINE4(futex_requeue,
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE2(set_robust_list,
- struct compat_robust_list_head __user *, head,
+ struct robust_list_head32 __user *, head,
compat_size_t, len)
{
if (unlikely(len != sizeof(*head)))
@@ -455,7 +455,7 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
compat_uptr_t __user *, head_ptr,
compat_size_t __user *, len_ptr)
{
- struct compat_robust_list_head __user *head;
+ struct robust_list_head32 __user *head;
unsigned long ret;
struct task_struct *p;
--
2.47.1
^ permalink raw reply related
* [PATCH v3 3/3] futex: Wire up set_robust_list2 syscall
From: André Almeida @ 2024-12-17 17:49 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, Nathan Chancellor,
Vinicius Peixoto, André Almeida
In-Reply-To: <20241217174958.477692-1-andrealmeid@igalia.com>
Wire up the new set_robust_list2 syscall in all available architectures.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
kernel/sys_ni.c | 1 +
scripts/syscall.tbl | 1 +
17 files changed, 17 insertions(+)
diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index c59d53d6d3f3..d1193a7f948e 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -506,3 +506,4 @@
574 common getxattrat sys_getxattrat
575 common listxattrat sys_listxattrat
576 common removexattrat sys_removexattrat
+577 common set_robust_list2 sys_robust_list2
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 49eeb2ad8dbd..269721f54a5c 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -481,3 +481,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f5ed71f1910d..75a387585b3a 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -466,3 +466,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 680f568b77f2..176f84b79c1c 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -472,3 +472,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 0b9b7e25b69a..47e28d67ca8a 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -405,3 +405,4 @@
464 n32 getxattrat sys_getxattrat
465 n32 listxattrat sys_listxattrat
466 n32 removexattrat sys_removexattrat
+467 n32 set_robust_list2 sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index c844cd5cda62..488c1bca7715 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -381,3 +381,4 @@
464 n64 getxattrat sys_getxattrat
465 n64 listxattrat sys_listxattrat
466 n64 removexattrat sys_removexattrat
+467 n64 set_robust_list2 sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 349b8aad1159..f983086695a8 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -454,3 +454,4 @@
464 o32 getxattrat sys_getxattrat
465 o32 listxattrat sys_listxattrat
466 o32 removexattrat sys_removexattrat
+467 o32 set_robust_list2 sys_set_robust_list2
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index d9fc94c86965..f8735cb8046b 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -465,3 +465,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index d8b4ab78bef0..1da55a6a3bb5 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -557,3 +557,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index e9115b4d8b63..93bda0d6580b 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -469,3 +469,4 @@
464 common getxattrat sys_getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2 sys_set_robust_list2
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c8cad33bf250..dd591da98af5 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -470,3 +470,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 727f99d333b3..a4ee76e234a3 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -512,3 +512,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 4d0fb2fba7e2..8d609abda75b 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -472,3 +472,4 @@
464 i386 getxattrat sys_getxattrat
465 i386 listxattrat sys_listxattrat
466 i386 removexattrat sys_removexattrat
+467 i386 set_robust_list2 sys_set_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5eb708bff1c7..2c6461df154b 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -390,6 +390,7 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 37effc1b134e..fa46635d7380 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -437,3 +437,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index c00a86931f8c..71fbac6176c8 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -195,6 +195,7 @@ COND_SYSCALL(move_pages);
COND_SYSCALL(set_mempolicy_home_node);
COND_SYSCALL(cachestat);
COND_SYSCALL(mseal);
+COND_SYSCALL(set_robust_list2);
COND_SYSCALL(perf_event_open);
COND_SYSCALL(accept4);
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index ebbdb3c42e9f..615a3043c982 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -407,3 +407,4 @@
464 common getxattrat sys_getxattrat
465 common listxattrat sys_listxattrat
466 common removexattrat sys_removexattrat
+467 common set_robust_list2 sys_set_robust_list2
--
2.47.1
^ permalink raw reply related
* [PATCH v3 2/3] futex: Create set_robust_list2
From: André Almeida @ 2024-12-17 17:49 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, Nathan Chancellor,
Vinicius Peixoto, André Almeida
In-Reply-To: <20241217174958.477692-1-andrealmeid@igalia.com>
Create a new robust_list() syscall. The current syscall can't be
expanded to cover the following use case, so a new one is needed. This
new syscall allows users to set multiple robust lists per process and to
have either 32bit or 64bit pointers in the list.
* Interface
This is the proposed interface:
long set_robust_list2(void *head, int index, unsigned int flags)
`head` is the head of the userspace struct robust_list_head, just as old
set_robust_list(). It needs to be a void pointer since it can point to a
normal robust_list_head or a compat_robust_list_head.
`flags` can be used for defining the list type:
enum robust_list_type {
ROBUST_LIST_32BIT,
ROBUST_LIST_64BIT,
};
`index` is the index in the internal robust_list's linked list (the
naming starts to get confusing, I reckon). If `index == -1`, that means
that user wants to set a new robust_list, and the kernel will append it
in the end of the list, assign a new index and return this index to the
user. If `index >= 0`, that means that user wants to re-set `*head` of
an already existing list (similarly to what happens when you call
set_robust_list() twice with different `*head`).
If `index` is out of range, or it points to a non-existing robust_list,
or if the internal list is full, an error is returned.
User cannot remove lists.
* Implementation
The old syscall's set/get_robust_list() are converted to use the linked
list as well. When using only the old syscalls user shouldn't any
difference as the internal code will handle the linked list insertion as
usual. When mixing old and new interfaces users should be aware that one
of the elements of the list was created by another syscall and they
should have special care handling this element index.
On exit, the linked list is parsed and all robust lists regardless of
which interface it was used to create them are handled.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
include/linux/futex.h | 5 +-
include/linux/sched.h | 5 +-
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/futex.h | 24 +++++++
kernel/futex/core.c | 111 ++++++++++++++++++++++++------
kernel/futex/futex.h | 5 ++
kernel/futex/syscalls.c | 78 +++++++++++++++++++--
7 files changed, 202 insertions(+), 30 deletions(-)
diff --git a/include/linux/futex.h b/include/linux/futex.h
index 8217b5ebdd9c..39335f21aea6 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -72,10 +72,11 @@ enum {
static inline void futex_init_task(struct task_struct *tsk)
{
- tsk->robust_list = NULL;
+ tsk->robust_list_index = -1;
#ifdef CONFIG_COMPAT
- tsk->compat_robust_list = NULL;
+ tsk->compat_robust_list_index = -1;
#endif
+ INIT_LIST_HEAD(&tsk->robust_list2);
INIT_LIST_HEAD(&tsk->pi_state_list);
tsk->pi_state_cache = NULL;
tsk->futex_state = FUTEX_STATE_OK;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index da573c16e93e..1324f103db3d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1290,10 +1290,11 @@ struct task_struct {
u32 rmid;
#endif
#ifdef CONFIG_FUTEX
- struct robust_list_head __user *robust_list;
+ int robust_list_index;
#ifdef CONFIG_COMPAT
- struct robust_list_head32 __user *compat_robust_list;
+ int compat_robust_list_index;
#endif
+ struct list_head robust_list2;
struct list_head pi_state_list;
struct futex_pi_state *pi_state_cache;
struct mutex futex_exit_mutex;
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 88dc393c2bca..477cce02ed72 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -850,8 +850,10 @@ __SYSCALL(__NR_listxattrat, sys_listxattrat)
#define __NR_removexattrat 466
__SYSCALL(__NR_removexattrat, sys_removexattrat)
+#define __NR_set_robust_list2 467
+
#undef __NR_syscalls
-#define __NR_syscalls 467
+#define __NR_syscalls 468
/*
* 32 bit systems traditionally used different
diff --git a/include/uapi/linux/futex.h b/include/uapi/linux/futex.h
index d2ee625ea189..13903a278b71 100644
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h
@@ -146,6 +146,30 @@ struct robust_list_head {
struct robust_list __user *list_op_pending;
};
+#define ROBUST_LISTS_PER_TASK 10
+
+enum robust_list2_type {
+ ROBUST_LIST_32BIT,
+ ROBUST_LIST_64BIT,
+};
+
+#define ROBUST_LIST_TYPE_MASK (ROBUST_LIST_32BIT | ROBUST_LIST_64BIT)
+
+/*
+ * This is an entry of a linked list of robust lists.
+ *
+ * @head: can point to a 64bit list or a 32bit list
+ * @list_type: determine the size of the futex pointers in the list
+ * @index: the index of this entry in the list
+ * @list: linked list element
+ */
+struct robust_list2_entry {
+ void __user *head;
+ enum robust_list2_type list_type;
+ unsigned int index;
+ struct list_head list;
+};
+
/*
* Are there any waiters for this robust futex:
*/
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index cc2d0f00cd6b..f8962bc27c90 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -775,9 +775,9 @@ static inline int fetch_robust_entry(struct robust_list __user **entry,
*
* We silently return on any sign of list-walking problem.
*/
-static void exit_robust_list64(struct task_struct *curr)
+static void exit_robust_list64(struct task_struct *curr,
+ struct robust_list_head __user *head)
{
- struct robust_list_head __user *head = curr->robust_list;
struct robust_list __user *entry, *next_entry, *pending;
unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
unsigned int next_pi;
@@ -837,7 +837,8 @@ static void exit_robust_list64(struct task_struct *curr)
}
}
#else
-static void exit_robust_list64(struct task_struct *curr)
+static void exit_robust_list64(struct task_struct *curr,
+ struct robust_list_head __user *head)
{
pr_warn("32bit kernel should not allow ROBUST_LIST_64BIT");
}
@@ -874,9 +875,9 @@ fetch_robust_entry32(u32 *uentry, struct robust_list __user **entry,
*
* We silently return on any sign of list-walking problem.
*/
-static void exit_robust_list32(struct task_struct *curr)
+static void exit_robust_list32(struct task_struct *curr,
+ struct robust_list_head32 __user *head)
{
- struct robust_list_head32 __user *head = curr->compat_robust_list;
struct robust_list __user *entry, *next_entry, *pending;
unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
unsigned int next_pi;
@@ -942,6 +943,70 @@ static void exit_robust_list32(struct task_struct *curr)
}
}
+long do_set_robust_list2(struct robust_list_head __user *head,
+ int index, unsigned int type)
+{
+ struct list_head *list2 = ¤t->robust_list2;
+ struct robust_list2_entry *prev, *new = NULL;
+
+ if (index == -1) {
+ if (list_empty(list2)) {
+ index = 0;
+ } else {
+ prev = list_last_entry(list2, struct robust_list2_entry, list);
+ index = prev->index + 1;
+ }
+
+ if (index >= ROBUST_LISTS_PER_TASK)
+ return -EINVAL;
+
+ new = kmalloc(sizeof(struct robust_list2_entry), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ list_add_tail(&new->list, list2);
+ new->index = index;
+
+ } else if (index >= 0) {
+ struct robust_list2_entry *curr;
+
+ if (list_empty(list2))
+ return -ENOENT;
+
+ list_for_each_entry(curr, list2, list) {
+ if (index == curr->index) {
+ new = curr;
+ break;
+ }
+ }
+
+ if (!new)
+ return -ENOENT;
+ }
+
+ BUG_ON(!new);
+ new->head = head;
+ new->list_type = type;
+
+ return index;
+}
+
+struct robust_list_head __user *get_robust_list2(int index)
+{
+ struct list_head *list2 = ¤t->robust_list2;
+ struct robust_list2_entry *curr;
+
+ if (list_empty(list2) || index == -1)
+ return NULL;
+
+ list_for_each_entry(curr, list2, list) {
+ if (index == curr->index)
+ return curr->head;
+ }
+
+ return NULL;
+}
+
#ifdef CONFIG_FUTEX_PI
/*
@@ -1023,24 +1088,28 @@ static inline void exit_pi_state_list(struct task_struct *curr) { }
static void futex_cleanup(struct task_struct *tsk)
{
-#ifdef CONFIG_64BIT
- if (unlikely(tsk->robust_list)) {
- exit_robust_list64(tsk);
- tsk->robust_list = NULL;
- }
-#else
- if (unlikely(tsk->robust_list)) {
- exit_robust_list32(tsk);
- tsk->robust_list = NULL;
- }
-#endif
+ struct robust_list2_entry *curr, *n;
+ struct list_head *list2 = &tsk->robust_list2;
-#ifdef CONFIG_COMPAT
- if (unlikely(tsk->compat_robust_list)) {
- exit_robust_list32(tsk);
- tsk->compat_robust_list = NULL;
+ /*
+ * Walk through the linked list, parsing robust lists and freeing the
+ * allocated lists
+ */
+ if (unlikely(!list_empty(list2))) {
+ list_for_each_entry_safe(curr, n, list2, list) {
+ if (curr->head != NULL) {
+ if (curr->list_type == ROBUST_LIST_64BIT)
+ exit_robust_list64(tsk, curr->head);
+ else if (curr->list_type == ROBUST_LIST_32BIT)
+ exit_robust_list32(tsk, curr->head);
+ curr->head = NULL;
+ }
+ list_del_init(&curr->list);
+ kfree(curr);
+ }
}
-#endif
+
+ tsk->robust_list_index = -1;
if (unlikely(!list_empty(&tsk->pi_state_list)))
exit_pi_state_list(tsk);
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 99b32e728c4a..07cbc9c1634a 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -404,6 +404,11 @@ extern int __futex_wait(u32 __user *uaddr, unsigned int flags, u32 val,
extern int futex_wait(u32 __user *uaddr, unsigned int flags, u32 val,
ktime_t *abs_time, u32 bitset);
+extern long do_set_robust_list2(struct robust_list_head __user *head,
+ int index, unsigned int type);
+
+extern struct robust_list_head __user *get_robust_list2(int index);
+
/**
* struct futex_vector - Auxiliary struct for futex_waitv()
* @w: Userspace provided data
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index dba193dfd216..b6fbaf8d9422 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -20,6 +20,18 @@
* the list. There can only be one such pending lock.
*/
+#ifdef CONFIG_64BIT
+static inline int robust_list_native_type(void)
+{
+ return ROBUST_LIST_64BIT;
+}
+#else
+static inline int robust_list_native_type(void)
+{
+ return ROBUST_LIST_32BIT;
+}
+#endif
+
/**
* sys_set_robust_list() - Set the robust-futex list head of a task
* @head: pointer to the list-head
@@ -28,17 +40,60 @@
SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
size_t, len)
{
+ unsigned int type = robust_list_native_type();
+ int ret;
+
/*
* The kernel knows only one size for now:
*/
if (unlikely(len != sizeof(*head)))
return -EINVAL;
- current->robust_list = head;
+ ret = do_set_robust_list2(head, current->robust_list_index, type);
+ if (ret < 0)
+ return ret;
+
+ current->robust_list_index = ret;
return 0;
}
+#define ROBUST_LIST_FLAGS ROBUST_LIST_TYPE_MASK
+
+/*
+ * sys_set_robust_list2()
+ *
+ * When index == -1, create a new list for user. When index >= 0, try to find
+ * the corresponding list and re-set the head there.
+ *
+ * Return values:
+ * >= 0: success, index of the robust list
+ * -EINVAL: invalid flags, invalid index
+ * -ENOENT: requested index no where to be found
+ * -ENOMEM: error allocating new list
+ * -ESRCH: too many allocated lists
+ */
+SYSCALL_DEFINE3(set_robust_list2, struct robust_list_head __user *, head,
+ int, index, unsigned int, flags)
+{
+ unsigned int type;
+
+ type = flags & ROBUST_LIST_TYPE_MASK;
+
+ if (index < -1 || index >= ROBUST_LISTS_PER_TASK)
+ return -EINVAL;
+
+ if ((flags & ~ROBUST_LIST_FLAGS) != 0)
+ return -EINVAL;
+
+#ifndef CONFIG_64BIT
+ if (type == ROBUST_LIST_64BIT)
+ return -EINVAL;
+#endif
+
+ return do_set_robust_list2(head, index, type);
+}
+
/**
* sys_get_robust_list() - Get the robust-futex list head of a task
* @pid: pid of the process [zero for current task]
@@ -52,6 +107,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
struct robust_list_head __user *head;
unsigned long ret;
struct task_struct *p;
+ int index;
rcu_read_lock();
@@ -68,9 +124,11 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
goto err_unlock;
- head = p->robust_list;
+ index = p->robust_list_index;
rcu_read_unlock();
+ head = get_robust_list2(index);
+
if (put_user(sizeof(*head), len_ptr))
return -EFAULT;
return put_user(head, head_ptr);
@@ -443,10 +501,19 @@ COMPAT_SYSCALL_DEFINE2(set_robust_list,
struct robust_list_head32 __user *, head,
compat_size_t, len)
{
+ unsigned int type = ROBUST_LIST_32BIT;
+ int ret;
+
if (unlikely(len != sizeof(*head)))
return -EINVAL;
- current->compat_robust_list = head;
+ ret = do_set_robust_list2((struct robust_list_head __user *) head,
+ current->robust_list_index, type);
+ if (ret < 0)
+ return ret;
+
+ current->robust_list_index = ret;
+
return 0;
}
@@ -458,6 +525,7 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
struct robust_list_head32 __user *head;
unsigned long ret;
struct task_struct *p;
+ int index;
rcu_read_lock();
@@ -474,9 +542,11 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
goto err_unlock;
- head = p->compat_robust_list;
+ index = p->compat_robust_list_index;
rcu_read_unlock();
+ head = (struct robust_list_head32 __user *) get_robust_list2(index);
+
if (put_user(sizeof(*head), len_ptr))
return -EFAULT;
return put_user(ptr_to_compat(head), head_ptr);
--
2.47.1
^ permalink raw reply related
* Re: [PATCH v3 3/3] futex: Wire up set_robust_list2 syscall
From: Arnd Bergmann @ 2024-12-17 19:24 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, Nathan Chancellor,
Vinicius Peixoto
In-Reply-To: <20241217174958.477692-4-andrealmeid@igalia.com>
On Tue, Dec 17, 2024, at 18:49, André Almeida wrote:
> Wire up the new set_robust_list2 syscall in all available architectures.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> arch/alpha/kernel/syscalls/syscall.tbl | 1 +
> arch/arm/tools/syscall.tbl | 1 +
Please also change arch/arm64/tools/syscall_32.tbl
Arnd
^ permalink raw reply
* Re: [PATCH v3 0/3] futex: Create set_robust_list2
From: Peter Zijlstra @ 2024-12-17 20:31 UTC (permalink / raw)
To: André Almeida
Cc: Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso,
Arnd Bergmann, sonicadvance1, linux-kernel, kernel-dev, linux-api,
Nathan Chancellor, Vinicius Peixoto
In-Reply-To: <20241217174958.477692-1-andrealmeid@igalia.com>
On Tue, Dec 17, 2024 at 02:49:55PM -0300, André Almeida wrote:
> This patch adds a new robust_list() syscall. The current syscall
> can't be expanded to cover the following use case, so a new one is
> needed. This new syscall allows users to set multiple robust lists per
> process and to have either 32bit or 64bit pointers in the list.
Last time a whole list of short comings of the current robust scheme
were laid bare. I feel we should address all that if we're going to
create a new scheme.
^ permalink raw reply
* Re: [PATCH v23 0/8] Script execution control (was O_MAYEXEC)
From: Mickaël Salaün @ 2024-12-18 10:40 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: 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>
On Thu, Dec 12, 2024 at 06:42:15PM +0100, Mickaël Salaün wrote:
> Hi,
>
> The goal of this patch series is to be able to ensure that direct file
> execution (e.g. ./script.sh) and indirect file execution (e.g. sh
> script.sh) lead to the same result, especially from a security point of
> view.
>
> The main changes from the previous version are the 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.
In the meantime I've pushed it in my tree, it should appear in -next
tomorrow. Please, let me know when you take it, I'll remove it from my
tree.
>
> Overview
> --------
>
> This patch series is a new approach of the initial O_MAYEXEC feature,
> and a revamp of the previous patch series. Taking into account the last
> reviews [1], we now stick to the kernel semantic for file executability.
> One major change is the clear split between access check and policy
> management.
>
> The first patch brings the AT_EXECVE_CHECK flag to execveat(2). The
> goal is to enable user space to check if a file could be executed (by
> the kernel). Unlike stat(2) that only checks file permissions,
> execveat2(2) + AT_EXECVE_CHECK take into account the full context,
> including mount points (noexec), caller's limits, and all potential LSM
> extra checks (e.g. argv, envp, credentials).
>
> The second patch brings two new securebits used to set or get a security
> policy for a set of processes. For this to be meaningful, all
> executable code needs to be trusted. In practice, this means that
> (malicious) users can be restricted to only run scripts provided (and
> trusted) by the system.
>
> [1] https://lore.kernel.org/r/CAHk-=wjPGNLyzeBMWdQu+kUdQLHQugznwY7CvWjmvNW47D5sog@mail.gmail.com
>
> Script execution
> ----------------
>
> One important thing to keep in mind is that the goal of this patch
> series is to get the same security restrictions with these commands:
> * ./script.py
> * python script.py
> * python < script.py
> * python -m script.py
>
> However, on secure systems, we should be able to forbid these commands
> because there is no way to reliably identify the origin of the script:
> * xargs -a script.py -d '\r' -- python -c
> * cat script.py | python
> * python
>
> Background
> ----------
>
> Compared to the previous patch series, there is no more dedicated
> syscall nor sysctl configuration. This new patch series only add new
> flags: one for execveat(2) and four for prctl(2).
>
> This kind of script interpreter restriction may already be used in
> hardened systems, which may need to fork interpreters and install
> different versions of the binaries. This mechanism should enable to
> avoid the use of duplicate binaries (and potential forked source code)
> for secure interpreters (e.g. secure Python [2]) by making it possible
> to dynamically enforce restrictions or not.
>
> The ability to control script execution is also required to close a
> major IMA measurement/appraisal interpreter integrity [3].
>
> This new execveat + AT_EXECVE_CHECK should not be confused with the
> O_EXEC flag (for open) which is intended for execute-only, which
> obviously doesn't work for scripts.
>
> I gave a talk about controlling script execution where I explain the
> previous approaches [4]. The design of the WIP RFC I talked about
> changed quite a bit since then.
>
> [2] https://github.com/zooba/spython
> [3] https://lore.kernel.org/lkml/20211014130125.6991-1-zohar@linux.ibm.com/
> [4] https://lssna2023.sched.com/event/1K7bO
>
> Execution policy
> ----------------
>
> The "execution" usage means that the content of the file descriptor is
> trusted according to the system policy to be executed by user space,
> which means that it interprets the content or (try to) maps it as
> executable memory.
>
> It is important to note that this can only enable to extend access
> control managed by the kernel. Hence it enables current access control
> mechanism to be extended and become a superset of what they can
> currently control. Indeed, the security policy could also be delegated
> to an LSM, either a MAC system or an integrity system.
>
> Complementary W^X protections can be brought by SELinux or IPE [5].
>
> Being able to restrict execution also enables to protect the kernel by
> restricting arbitrary syscalls that an attacker could perform with a
> crafted binary or certain script languages. It also improves multilevel
> isolation by reducing the ability of an attacker to use side channels
> with specific code. These restrictions can natively be enforced for ELF
> binaries (with the noexec mount option) but require this kernel
> extension to properly handle scripts (e.g. Python, Perl). To get a
> consistent execution policy, additional memory restrictions should also
> be enforced (e.g. thanks to SELinux).
>
> [5] https://lore.kernel.org/lkml/1716583609-21790-1-git-send-email-wufan@linux.microsoft.com/
>
> Prerequisite for security use
> -----------------------------
>
> Because scripts might not currently have the executable permission and
> still run well as is, or because we might want specific users to be
> allowed to run arbitrary scripts, we also need a configuration
> mechanism.
>
> According to the threat model, to get a secure execution environment on
> top of these changes, it might be required to configure and enable
> existing security mechanisms such as secure boot, restrictive mount
> points (e.g. with rw AND noexec), correct file permissions (including
> executable libraries), IMA/EVM, SELinux policy...
>
> The first thing to patch is the libc to check loaded libraries (e.g. see
> chromeOS changes). The second thing to patch are the script
> interpreters by checking direct scripts executability and by checking
> their own libraries (e.g. Python's imported files or argument-passed
> modules). For instance, the PEP 578 [6] (Runtime Audit Hooks) enables
> Python 3.8 to be extended with policy enforcement points related to code
> interpretation, which can be used to align with the PowerShell audit
> features. Additional Python security improvements (e.g. a limited
> interpreter without -c, stdin piping of code) are developed [2] [7].
>
> [6] https://www.python.org/dev/peps/pep-0578/
> [7] https://lore.kernel.org/lkml/0c70debd-e79e-d514-06c6-4cd1e021fa8b@python.org/
>
> libc patch
> ----------
>
> Dynamic linking needs still need to check the libraries the same way
> interpreters need to check scripts.
>
> chromeOS patches glibc with a fstatvfs check [8] [9]. This enables to
> check against noexec mount points, which is OK but doesn't fit with
> execve semantics. Moreover, the kernel is not aware of such check, so
> all access control checks are not performed (e.g. file permission, LSMs
> security policies, integrity and authenticity checks), it is not handled
> with audit, and more importantly this would not work on generic
> distributions because of the strict requirement and chromeOS-specific
> assumptions.
>
> [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
* Re: [PATCH v23 0/8] Script execution control (was O_MAYEXEC)
From: Kees Cook @ 2024-12-18 19:31 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Paul Moore, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Roberto Sassu, Scott Shell,
Shuah Khan, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, kernel-hardening, linux-api,
linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241218.aBaituy0veK7@digikod.net>
On Wed, Dec 18, 2024 at 11:40:59AM +0100, Mickaël Salaün wrote:
> In the meantime I've pushed it in my tree, it should appear in -next
> tomorrow. Please, let me know when you take it, I'll remove it from my
> tree.
Thanks! Yeah, I was just finally getting through my email after my
pre-holiday holiday. ;)
I'll get this into my -next tree now.
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v23 0/8] Script execution control (was O_MAYEXEC)
From: Mickaël Salaün @ 2024-12-19 7:44 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Christian Brauner, Paul Moore, Serge Hallyn,
Adhemerval Zanella Netto, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Eric Biggers,
Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
James Morris, Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Linus Torvalds,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Roberto Sassu, Scott Shell,
Shuah Khan, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, kernel-hardening, linux-api,
linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <202412181130.84A2FCF2@keescook>
On Wed, Dec 18, 2024 at 11:31:57AM -0800, Kees Cook wrote:
> On Wed, Dec 18, 2024 at 11:40:59AM +0100, Mickaël Salaün wrote:
> > In the meantime I've pushed it in my tree, it should appear in -next
> > tomorrow. Please, let me know when you take it, I'll remove it from my
> > tree.
>
> Thanks! Yeah, I was just finally getting through my email after my
> pre-holiday holiday. ;)
>
> I'll get this into my -next tree now.
Thanks, I just removed mine.
>
> -Kees
>
> --
> Kees Cook
^ permalink raw reply
* Re: [PATCH v3 0/3] futex: Create set_robust_list2
From: André Almeida @ 2024-12-19 14:28 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso,
Arnd Bergmann, sonicadvance1, linux-kernel, kernel-dev, linux-api,
Nathan Chancellor, Vinicius Peixoto
In-Reply-To: <20241217203140.GH11133@noisy.programming.kicks-ass.net>
Em 17/12/2024 17:31, Peter Zijlstra escreveu:
> On Tue, Dec 17, 2024 at 02:49:55PM -0300, André Almeida wrote:
>> This patch adds a new robust_list() syscall. The current syscall
>> can't be expanded to cover the following use case, so a new one is
>> needed. This new syscall allows users to set multiple robust lists per
>> process and to have either 32bit or 64bit pointers in the list.
>
> Last time a whole list of short comings of the current robust scheme
> were laid bare. I feel we should address all that if we're going to
> create a new scheme.
>
Are you talking about [1] or is there something else?
[1] https://lore.kernel.org/lkml/87jzdjxjj8.fsf@oldenburg3.str.redhat.com/
^ permalink raw reply
* Re: [PATCH v3 0/3] futex: Create set_robust_list2
From: Peter Zijlstra @ 2024-12-19 17:13 UTC (permalink / raw)
To: André Almeida
Cc: Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso,
Arnd Bergmann, sonicadvance1, linux-kernel, kernel-dev, linux-api,
Nathan Chancellor, Vinicius Peixoto, fweimer, Mathieu Desnoyers
In-Reply-To: <a9b1dde0-7c29-41c3-99be-4749281e25ea@igalia.com>
On Thu, Dec 19, 2024 at 11:28:27AM -0300, André Almeida wrote:
> Em 17/12/2024 17:31, Peter Zijlstra escreveu:
> > On Tue, Dec 17, 2024 at 02:49:55PM -0300, André Almeida wrote:
> > > This patch adds a new robust_list() syscall. The current syscall
> > > can't be expanded to cover the following use case, so a new one is
> > > needed. This new syscall allows users to set multiple robust lists per
> > > process and to have either 32bit or 64bit pointers in the list.
> >
> > Last time a whole list of short comings of the current robust scheme
> > were laid bare. I feel we should address all that if we're going to
> > create a new scheme.
> >
>
> Are you talking about [1] or is there something else?
>
> [1] https://lore.kernel.org/lkml/87jzdjxjj8.fsf@oldenburg3.str.redhat.com/
Correct, that thread.
So at the very least I think we should enforce natural alignment of the
robust entry -- this ensures the whole object is always on a single
page. This should then allow emulators (like QEMU) to convert things
back to native address space.
Additionally, I think we can replace the LIST_LIMIT -- whoes purpose is
to mitigate the danger of loops -- with the kernel simply destroying the
list while it iterates it. That way it cannot be caught in loops, no
matter what userspace did.
That then leaves the whole munmap() race -- and I'm not really sure what
to do about that one. I did outline two option, but they're both quite
terrible.
The mmap()/munmap() code would need to serialize against list_op_pending
without incurring undue overhead in the common case.
Ideally we make the whole thing using RSEQ such that list_op_pending
becomes atomic vs preemption -- but I've not thought that through.
^ permalink raw reply
* [RFC PATCH 0/2] ptp: add PTP_SYS_OFFSET_STAT ioctl, support it in virtio_rtc
From: Peter Hilber @ 2024-12-19 20:42 UTC (permalink / raw)
To: linux-kernel, virtualization, virtio-dev, netdev
Cc: Trilok Soni, Srivatsa Vaddagiri, Peter Hilber, David S. Miller,
Eugenio Pérez, Michael S. Tsirkin, Andrew Lunn, Eric Dumazet,
Jakub Kicinski, Jason Wang, Paolo Abeni, Richard Cochran,
Shuah Khan, Xuan Zhuo, linux-kselftest, linux-api,
David Woodhouse, Ridoux, Julien, John Stultz, Thomas Gleixner,
Stephen Boyd, Anna-Maria Behnsen
This RFC patch series proposes a new ioctl PTP_SYS_OFFSET_STAT and adds
support for it in the proposed virtio_rtc driver [1]. The new
PTP_SYS_OFFSET_STAT ioctl provides a cross-timestamp like
PTP_SYS_OFFSET_PRECISE2, plus any the following status information (for
now):
- for UTC timescale clocks: leap second related status,
- clock accuracy.
The second commit adds support for the ioctl in the proposed virtio_rtc
driver, and hence depends on the patch series "Add virtio_rtc module" [1].
[1] https://lore.kernel.org/lkml/20241219201118.2233-1-quic_philber@quicinc.com/T/#t
Signed-off-by: Peter Hilber <quic_philber@quicinc.com>
Peter Hilber (2):
ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
virtio_rtc: Support PTP_SYS_OFFSET_STAT ioctl
drivers/ptp/ptp_chardev.c | 39 ++++++++
drivers/ptp/ptp_clock.c | 9 ++
drivers/virtio/Kconfig | 4 +-
drivers/virtio/virtio_rtc_driver.c | 122 +++++++++++++++++++++++-
drivers/virtio/virtio_rtc_internal.h | 3 +-
drivers/virtio/virtio_rtc_ptp.c | 25 +++--
include/linux/ptp_clock_kernel.h | 31 ++++++
include/uapi/linux/ptp_clock.h | 130 +++++++++++++++++++++++++-
tools/testing/selftests/ptp/Makefile | 2 +-
tools/testing/selftests/ptp/testptp.c | 126 ++++++++++++++++++++++++-
10 files changed, 471 insertions(+), 20 deletions(-)
base-commit: 8a8009abbfa04e58f1b01b20534cac9e8fe61a46
--
2.43.0
^ permalink raw reply
* [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Peter Hilber @ 2024-12-19 20:42 UTC (permalink / raw)
To: linux-kernel, virtualization, virtio-dev, netdev
Cc: Trilok Soni, Srivatsa Vaddagiri, Peter Hilber, David S. Miller,
Eugenio Pérez, Michael S. Tsirkin, Andrew Lunn, Eric Dumazet,
Jakub Kicinski, Jason Wang, Paolo Abeni, Richard Cochran,
Shuah Khan, Xuan Zhuo, linux-kselftest, linux-api,
David Woodhouse, Ridoux, Julien, John Stultz, Thomas Gleixner,
Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <20241219204208.3160-1-quic_philber@quicinc.com>
Ioctl PTP_SYS_OFFSET_PRECISE2 provides cross-timestamping of device time
and system time. This can be used for virtualization where (virtualization)
host and guest refer to the same clocksource. It may be preferred to
indicate UTC time, rather than TAI. It is then useful to indicate when and
how the host processes UTC leap seconds (stepping or smearing on leap
seconds), in particular if the guest is not guaranteed to have an
up-to-date Time Zone Database or similar.
Also, a host may have a notion of how accurate its clock is w.r.t. the
hosts' reference clocks.
Add a new ioctl PTP_SYS_OFFSET_STAT, which can convey, in addition to the
cross-timestamp:
- leap second related status,
- clock accuracy.
Reserve space for more information.
Drivers indicate through flags which status information is valid. A driver
zeroing struct ptp_stat_extra would only provide the
PTP_SYS_OFFSET_PRECISE2 information. Drivers implementing the backing
getstattstamp op will automatically also support PTP_SYS_OFFSET_PRECISE2.
Indicate support of the new ioctl in the capability ioctl
(PTP_CLOCK_GETCAPS2). Extend the testptp program with the new ioctl.
Users
=====
The status information corresponds to the information conveyed in the
virtio-rtc spec RFC v6 [1].
The RFC virtio_rtc driver implements the getstattstamp op backing the new
ioctl. I locally patched the chrony time sync daemon to process both the
leap second indication (when leap seconds are not smeared) and the TAI
offset indication.
The other status information is not yet used by general-purpose tools, but
can be helpful for custom use cases.
There is even more clock status information which might be added [2] to
virtio_rtc, but which has not been formally proposed so far.
Discussion
==========
This is intended to solicit comments about
- which status information is useful to third parties (in particular w.r.t.
leap second smearing and clock accuracy)
- the most appropriate format to expose status information for
virtualization related PHCs.
I am aware that adding support for independent PTP clocks is in progress,
but couldn't yet see any clash with this.
Alternatives
------------
An essential part of the status information could alternatively be conveyed
through clock_adjtime(2). However, struct timex should then be extended
with a system clock timestamp, which would eat a lot of padding bytes.
PTP_SYS_OFFSET_PRECISE2 only has 16 reserved bytes, so could at most add
leap second related information, which does appear unneeded for not
virtualization related use cases.
Splitting up time and leap second status retrieval to multiple syscalls
might lead to multiple requests to the device, and to race conditions.
TBD
===
- Dissect new ioctl flags in testptp.
[1] https://lore.kernel.org/virtio-comment/20241206094819.858-1-quic_philber@quicinc.com/T/#t
[2] cf. struct vmclock_abi in https://lore.kernel.org/all/78969a39b51ec00e85551b752767be65f6794b46.camel@infradead.org/
Signed-off-by: Peter Hilber <quic_philber@quicinc.com>
---
drivers/ptp/ptp_chardev.c | 39 ++++++++
drivers/ptp/ptp_clock.c | 9 ++
include/linux/ptp_clock_kernel.h | 31 ++++++
include/uapi/linux/ptp_clock.h | 130 +++++++++++++++++++++++++-
tools/testing/selftests/ptp/Makefile | 2 +-
tools/testing/selftests/ptp/testptp.c | 126 ++++++++++++++++++++++++-
6 files changed, 331 insertions(+), 6 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index ea96a14d72d1..c87062a5a568 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -165,8 +165,10 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
struct ptp_sys_offset_precise precise_offset;
struct system_device_crosststamp xtstamp;
struct ptp_clock_info *ops = ptp->info;
+ struct ptp_sys_offset_stat stat_offset;
struct ptp_sys_offset *sysoff = NULL;
struct timestamp_event_queue *tsevq;
+ struct ptp_stat_extra stat_extra;
struct ptp_system_timestamp sts;
struct ptp_clock_request req;
struct ptp_clock_caps caps;
@@ -195,6 +197,7 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
ptp->info->getmaxphase != NULL;
if (caps.adjust_phase)
caps.max_phase_adj = ptp->info->getmaxphase(ptp->info);
+ caps.stat_timestamping = ptp->info->getstattstamp != NULL;
if (copy_to_user((void __user *)arg, &caps, sizeof(caps)))
err = -EFAULT;
break;
@@ -347,6 +350,42 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
err = -EFAULT;
break;
+ case PTP_SYS_OFFSET_STAT:
+ if (!ptp->info->getstattstamp) {
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ memset(&stat_extra, 0, sizeof(stat_extra));
+
+ err = ptp->info->getstattstamp(ptp->info, &xtstamp,
+ &stat_extra);
+ if (err)
+ break;
+
+ memset(&stat_offset, 0, sizeof(stat_offset));
+
+ ts = ktime_to_timespec64(xtstamp.device);
+ stat_offset.device.sec = ts.tv_sec;
+ stat_offset.device.nsec = ts.tv_nsec;
+ ts = ktime_to_timespec64(xtstamp.sys_realtime);
+ stat_offset.sys_realtime.sec = ts.tv_sec;
+ stat_offset.sys_realtime.nsec = ts.tv_nsec;
+ ts = ktime_to_timespec64(xtstamp.sys_monoraw);
+ stat_offset.sys_monoraw.sec = ts.tv_sec;
+ stat_offset.sys_monoraw.nsec = ts.tv_nsec;
+
+ stat_offset.accuracy = stat_extra.accuracy;
+ stat_offset.flags = stat_extra.flags;
+ stat_offset.leap = stat_extra.leap;
+ stat_offset.tai_offset_sec = stat_extra.tai_offset_sec;
+ stat_offset.smear_offset_nsec = stat_extra.smear_offset_nsec;
+
+ if (copy_to_user((void __user *)arg, &stat_offset,
+ sizeof(stat_offset)))
+ err = -EFAULT;
+ break;
+
case PTP_SYS_OFFSET_EXTENDED:
case PTP_SYS_OFFSET_EXTENDED2:
if (!ptp->info->gettimex64) {
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index b932425ddc6a..3b08d133c2bb 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -209,6 +209,12 @@ static void ptp_clock_release(struct device *dev)
kfree(ptp);
}
+static int ptp_getcrosststamp_from_stat(struct ptp_clock_info *info,
+ struct system_device_crosststamp *cts)
+{
+ return info->getstattstamp(info, cts, NULL);
+}
+
static int ptp_getcycles64(struct ptp_clock_info *info, struct timespec64 *ts)
{
if (info->getcyclesx64)
@@ -279,6 +285,9 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
mutex_init(&ptp->n_vclocks_mux);
init_waitqueue_head(&ptp->tsev_wq);
+ if (ptp->info->getstattstamp && !ptp->info->getcrosststamp)
+ ptp->info->getcrosststamp = ptp_getcrosststamp_from_stat;
+
if (ptp->info->getcycles64 || ptp->info->getcyclesx64) {
ptp->has_cycles = true;
if (!ptp->info->getcycles64 && ptp->info->getcyclesx64)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 0d68d09bedd1..9b08cbd747ae 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -55,6 +55,25 @@ struct ptp_system_timestamp {
clockid_t clockid;
};
+/**
+ * struct ptp_stat_extra - extra clock status
+ *
+ * @accuracy: Accuracy of the clock at the time of reading.
+ * @flags: Same flags as struct ptp_sys_offset_stat.
+ * @leap: One of the ptp_utc_leap enumeration values.
+ * @tai_offset_sec: Offset of TAI from UTC at the time of clock reading,
+ * in the form TAI - UTC.
+ * @smear_offset_nsec: Offset of smeared clock from UTC at the time of
+ * clock reading, in the form UTC - smeared clock.
+ */
+struct ptp_stat_extra {
+ struct ptp_clock_accuracy accuracy;
+ unsigned int flags;
+ unsigned int leap;
+ int tai_offset_sec;
+ int smear_offset_nsec;
+};
+
/**
* struct ptp_clock_info - describes a PTP hardware clock
*
@@ -109,6 +128,15 @@ struct ptp_system_timestamp {
* parameter cts: Contains timestamp (device,system) pair,
* where system time is realtime and monotonic.
*
+ * @getstattstamp: Reads the current time from the hardware clock and system
+ * clock simultaneously, optionally with extra status
+ * information.
+ * parameter cts: Contains timestamp (device,system) pair,
+ * where system time is realtime and monotonic.
+ * parameter stat_extra: If not NULL, it holds extra status
+ * information. Drivers indicate information validity through
+ * flags.
+ *
* @settime64: Set the current time on the hardware clock.
* parameter ts: Time value to set.
*
@@ -184,6 +212,9 @@ struct ptp_clock_info {
struct ptp_system_timestamp *sts);
int (*getcrosststamp)(struct ptp_clock_info *ptp,
struct system_device_crosststamp *cts);
+ int (*getstattstamp)(struct ptp_clock_info *ptp,
+ struct system_device_crosststamp *cts,
+ struct ptp_stat_extra *stat_extra);
int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
int (*getcycles64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
int (*getcyclesx64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 18eefa6d93d6..c26f48967b41 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -103,7 +103,12 @@ struct ptp_clock_caps {
/* Whether the clock supports adjust phase */
int adjust_phase;
int max_phase_adj; /* Maximum phase adjustment in nanoseconds. */
- int rsv[11]; /* Reserved for future use. */
+ /*
+ * Whether the clock supports precise system-device cross timestamps
+ * with extra status information.
+ */
+ int stat_timestamping;
+ int rsv[10]; /* Reserved for future use. */
};
struct ptp_extts_request {
@@ -184,6 +189,128 @@ struct ptp_sys_offset_precise {
unsigned int rsv[4]; /* Reserved for future use. */
};
+/*
+ * Bits of the ptp_clock_accuracy.flags field:
+ */
+#define PTP_CLOCK_FREQ_EST_VALID (1 << 0)
+#define PTP_CLOCK_FREQ_MAX_VALID (1 << 1)
+#define PTP_CLOCK_TIME_EST_VALID (1 << 2)
+#define PTP_CLOCK_TIME_MAX_VALID (1 << 3)
+
+enum ptp_clock_status {
+ PTP_CLOCK_STATUS_UNKNOWN,
+ PTP_CLOCK_STATUS_INITIALIZING,
+ PTP_CLOCK_STATUS_SYNCHRONIZED,
+ PTP_CLOCK_STATUS_FREERUNNING,
+ PTP_CLOCK_STATUS_UNRELIABLE,
+};
+
+struct ptp_clock_accuracy {
+ /*
+ * Estimated (absolute) drift rate, in ppm, with a 44-bit fractional
+ * part. Field value 1 corresponds to 2^-44 ppm.
+ *
+ * Valid if flag PTP_CLOCK_FREQ_EST_VALID is set.
+ */
+ unsigned long long freq_esterror;
+ /*
+ * Maximum (absolute) drift rate, in ppm, with a 44-bit fractional
+ * part.
+ *
+ * Valid if flag PTP_CLOCK_FREQ_MAX_VALID is set.
+ */
+ unsigned long long freq_maxerror;
+ /*
+ * Estimate of absolute value of offset from reference, in nsec.
+ *
+ * Valid if flag PTP_CLOCK_TIME_EST_VALID is set.
+ */
+ unsigned long long time_esterror;
+ /*
+ * Maximum of absolute value of offset from reference, in nsec.
+ *
+ * Valid if flag PTP_CLOCK_TIME_MAX_VALID is set.
+ */
+ unsigned long long time_maxerror;
+ /* PTP_CLOCK_(FREQ|TIME)_*_VALID flags. */
+ unsigned int flags;
+ /* PTP_CLOCK_STATUS_* enum value. */
+ unsigned int clock_status;
+};
+
+/*
+ * Bits of the ptp_sys_offset_stat.flags field:
+ */
+#define PTP_CLOCK_LEAP_VALID (1 << 0)
+#define PTP_CLOCK_TAI_OFFSET_VALID (1 << 1)
+#define PTP_CLOCK_SMEAR_OFFSET_VALID (1 << 2)
+
+enum ptp_utc_leap {
+ /* No leap second in the present, near future, or near past. */
+ PTP_LEAP_NONE,
+ /* Positive leap second happens in the near future. */
+ PTP_LEAP_PRE_POS,
+ /* Negative leap second happens in the near future. */
+ PTP_LEAP_PRE_NEG,
+ /* Positive leap second is in progress. */
+ PTP_LEAP_POS,
+ /* Positive leap second happened recently. */
+ PTP_LEAP_POST_POS,
+ /* Negative leap second happened recently. */
+ PTP_LEAP_POST_NEG,
+ /*
+ * Positive leap second will be smeared into the clock in the near
+ * future.
+ */
+ PTP_LEAP_SMEAR_PRE_POS,
+ /*
+ * Negative leap second will be smeared into the clock in the near
+ * future.
+ */
+ PTP_LEAP_SMEAR_PRE_NEG,
+ /* Positive leap second is being smeared into the clock. */
+ PTP_LEAP_SMEAR_POS,
+ /* Negative leap second is being smeared into the clock. */
+ PTP_LEAP_SMEAR_NEG,
+ /* Positive leap second was smeared into the clock recently. */
+ PTP_LEAP_SMEAR_POST_POS,
+ /* Negative leap second was smeared into the clock recently. */
+ PTP_LEAP_SMEAR_POST_NEG,
+};
+
+/*
+ * Precise system-device cross timestamp with extra info.
+ */
+struct ptp_sys_offset_stat {
+ struct ptp_clock_time device;
+ struct ptp_clock_time sys_realtime;
+ struct ptp_clock_time sys_monoraw;
+ struct ptp_clock_accuracy accuracy;
+ unsigned int flags;
+ /*
+ * One of the ptp_utc_leap enumeration values.
+ *
+ * Valid if flag PTP_CLOCK_LEAP_VALID is set.
+ */
+ unsigned int leap;
+ /*
+ * Offset of TAI from UTC at the time of clock reading, in the form
+ * TAI - UTC.
+ *
+ * Valid if flag PTP_CLOCK_TAI_OFFSET_VALID is set.
+ */
+ int tai_offset_sec;
+ /*
+ * Offset of smeared clock from UTC at the time of clock reading, in
+ * the form UTC - smeared clock.
+ *
+ * Valid if flag PTP_CLOCK_SMEAR_OFFSET_VALID is set.
+ */
+ int smear_offset_nsec;
+ /* Reserved for future use. */
+ unsigned int rsv[22];
+};
+
enum ptp_pin_function {
PTP_PF_NONE,
PTP_PF_EXTTS,
@@ -245,6 +372,7 @@ struct ptp_pin_desc {
_IOWR(PTP_CLK_MAGIC, 18, struct ptp_sys_offset_extended)
#define PTP_MASK_CLEAR_ALL _IO(PTP_CLK_MAGIC, 19)
#define PTP_MASK_EN_SINGLE _IOW(PTP_CLK_MAGIC, 20, unsigned int)
+#define PTP_SYS_OFFSET_STAT _IOWR(PTP_CLK_MAGIC, 21, struct ptp_sys_offset_stat)
struct ptp_extts_event {
struct ptp_clock_time t; /* Time event occurred. */
diff --git a/tools/testing/selftests/ptp/Makefile b/tools/testing/selftests/ptp/Makefile
index 8f57f88ecadd..3fa960c72337 100644
--- a/tools/testing/selftests/ptp/Makefile
+++ b/tools/testing/selftests/ptp/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += $(KHDR_INCLUDES)
TEST_GEN_PROGS := testptp
-LDLIBS += -lrt
+LDLIBS += -lrt -lm
TEST_PROGS = phc.sh
include ../lib.mk
diff --git a/tools/testing/selftests/ptp/testptp.c b/tools/testing/selftests/ptp/testptp.c
index 58064151f2c8..7d09f4b9272e 100644
--- a/tools/testing/selftests/ptp/testptp.c
+++ b/tools/testing/selftests/ptp/testptp.c
@@ -37,6 +37,8 @@
#define NSEC_PER_SEC 1000000000LL
+#define STAT_FRACTIONAL_BITS 44
+
/* clock_adjtime is not available in GLIBC < 2.14 */
#if !__GLIBC_PREREQ(2, 14)
#include <sys/syscall.h>
@@ -147,6 +149,7 @@ static void usage(char *progname)
" -x val get an extended ptp clock time with the desired number of samples (up to %d)\n"
" -X get a ptp clock cross timestamp\n"
" -y val pre/post tstamp timebase to use {realtime|monotonic|monotonic-raw}\n"
+ " -Y get a ptp clock cross timestamp with extra status information\n"
" -z test combinations of rising/falling external time stamp flags\n",
progname, PTP_MAX_SAMPLES);
}
@@ -164,6 +167,8 @@ int main(int argc, char *argv[])
struct ptp_sys_offset *sysoff;
struct ptp_sys_offset_extended *soe;
struct ptp_sys_offset_precise *xts;
+ struct ptp_sys_offset_stat *sts;
+ struct ptp_clock_accuracy *accuracy;
char *progname;
unsigned int i;
@@ -184,6 +189,7 @@ int main(int argc, char *argv[])
int pct_offset = 0;
int getextended = 0;
int getcross = 0;
+ int getstat = 0;
int n_samples = 0;
int pin_index = -1, pin_func;
int pps = -1;
@@ -191,6 +197,7 @@ int main(int argc, char *argv[])
int settime = 0;
int channel = -1;
clockid_t ext_clockid = CLOCK_REALTIME;
+ double freq_ppm_mult;
int64_t t1, t2, tp;
int64_t interval, offset;
@@ -200,7 +207,7 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
- while (EOF != (c = getopt(argc, argv, "cd:e:f:F:ghH:i:k:lL:n:o:p:P:sSt:T:w:x:Xy:z"))) {
+ while (EOF != (c = getopt(argc, argv, "cd:e:f:F:ghH:i:k:lL:n:o:p:P:sSt:T:w:x:Xy:Yz"))) {
switch (c) {
case 'c':
capabilities = 1;
@@ -294,7 +301,9 @@ int main(int argc, char *argv[])
return -1;
}
break;
-
+ case 'Y':
+ getstat = 1;
+ break;
case 'z':
flagtest = 1;
break;
@@ -333,7 +342,8 @@ int main(int argc, char *argv[])
" %d programmable pins\n"
" %d cross timestamping\n"
" %d adjust_phase\n"
- " %d maximum phase adjustment (ns)\n",
+ " %d maximum phase adjustment (ns)\n"
+ " %d cross timestamping with extra status\n",
caps.max_adj,
caps.n_alarm,
caps.n_ext_ts,
@@ -342,7 +352,8 @@ int main(int argc, char *argv[])
caps.n_pins,
caps.cross_timestamping,
caps.adjust_phase,
- caps.max_phase_adj);
+ caps.max_phase_adj,
+ caps.stat_timestamping);
}
}
@@ -661,6 +672,113 @@ int main(int argc, char *argv[])
free(xts);
}
+ if (getstat) {
+ sts = calloc(1, sizeof(*sts));
+ if (!sts) {
+ perror("calloc");
+ return -1;
+ }
+
+ if (ioctl(fd, PTP_SYS_OFFSET_STAT, sts)) {
+ perror("PTP_SYS_OFFSET_STAT");
+ } else {
+ puts("system and phc crosstimestamping with extra status request okay");
+
+ printf("device time: %lld.%09u\n",
+ sts->device.sec, sts->device.nsec);
+ printf("system time: %lld.%09u\n",
+ sts->sys_realtime.sec, sts->sys_realtime.nsec);
+ printf("monoraw time: %lld.%09u\n",
+ sts->sys_monoraw.sec, sts->sys_monoraw.nsec);
+
+ accuracy = &sts->accuracy;
+ freq_ppm_mult = pow(2, -STAT_FRACTIONAL_BITS);
+ printf("est. frequency error: %e ppm\n",
+ accuracy->freq_esterror * freq_ppm_mult);
+ printf("max. frequency error: %e ppm\n",
+ accuracy->freq_maxerror * freq_ppm_mult);
+ printf("est. time error: %llu.%09llu\n",
+ accuracy->time_esterror / NSEC_PER_SEC,
+ accuracy->time_esterror % NSEC_PER_SEC);
+ printf("max. time error: %llu.%09llu\n",
+ accuracy->time_maxerror / NSEC_PER_SEC,
+ accuracy->time_maxerror % NSEC_PER_SEC);
+ printf("accuracy flags: 0x%x\n", accuracy->flags);
+
+ switch (accuracy->clock_status) {
+ case PTP_CLOCK_STATUS_UNKNOWN:
+ printf("status: unknown\n");
+ break;
+ case PTP_CLOCK_STATUS_INITIALIZING:
+ printf("status: initializing\n");
+ break;
+ case PTP_CLOCK_STATUS_SYNCHRONIZED:
+ printf("status: synchronized\n");
+ break;
+ case PTP_CLOCK_STATUS_FREERUNNING:
+ printf("status: free-running\n");
+ break;
+ case PTP_CLOCK_STATUS_UNRELIABLE:
+ printf("status: unreliable\n");
+ break;
+ default:
+ printf("status: ? (0x%x)\n",
+ accuracy->clock_status);
+ break;
+ }
+
+ printf("flags: 0x%x\n", sts->flags);
+ switch (sts->leap) {
+ case PTP_LEAP_NONE:
+ printf("leap second: none\n");
+ break;
+ case PTP_LEAP_PRE_POS:
+ printf("leap second: pre-announcing positive leap second\n");
+ break;
+ case PTP_LEAP_PRE_NEG:
+ printf("leap second: pre-announcing negative leap second\n");
+ break;
+ case PTP_LEAP_POS:
+ printf("leap second: positive leap second in progress\n");
+ break;
+ case PTP_LEAP_POST_POS:
+ printf("leap second: past positive leap second\n");
+ break;
+ case PTP_LEAP_POST_NEG:
+ printf("leap second: past negative leap second\n");
+ break;
+ case PTP_LEAP_SMEAR_PRE_POS:
+ printf("leap second: pre-announcing smearing for positive leap second\n");
+ break;
+ case PTP_LEAP_SMEAR_PRE_NEG:
+ printf("leap second: pre-announcing smearing for negative leap second\n");
+ break;
+ case PTP_LEAP_SMEAR_POS:
+ printf("leap second: smearing for positive leap second in progress\n");
+ break;
+ case PTP_LEAP_SMEAR_NEG:
+ printf("leap second: smearing for negative leap second in progress\n");
+ break;
+ case PTP_LEAP_SMEAR_POST_POS:
+ printf("leap second: past smearing for positive leap second\n");
+ break;
+ case PTP_LEAP_SMEAR_POST_NEG:
+ printf("leap second: past smearing for negative leap second\n");
+ break;
+ default:
+ printf("leap second: ? (0x%x)\n", sts->leap);
+ break;
+ }
+ printf("tai offset: %d\n", sts->tai_offset_sec);
+ printf("smear offset: %s%d.%09d\n",
+ sts->smear_offset_nsec < 0 ? "-" : "",
+ abs(sts->smear_offset_nsec) / (int)NSEC_PER_SEC,
+ abs(sts->smear_offset_nsec) % (int)NSEC_PER_SEC);
+ }
+
+ free(sts);
+ }
+
if (channel >= 0) {
if (ioctl(fd, PTP_MASK_CLEAR_ALL)) {
perror("PTP_MASK_CLEAR_ALL");
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 2/2] virtio_rtc: Support PTP_SYS_OFFSET_STAT ioctl
From: Peter Hilber @ 2024-12-19 20:42 UTC (permalink / raw)
To: linux-kernel, virtualization, virtio-dev, netdev
Cc: Trilok Soni, Srivatsa Vaddagiri, Peter Hilber, David S. Miller,
Eugenio Pérez, Michael S. Tsirkin, Andrew Lunn, Eric Dumazet,
Jakub Kicinski, Jason Wang, Paolo Abeni, Richard Cochran,
Shuah Khan, Xuan Zhuo, linux-kselftest, linux-api,
David Woodhouse, Ridoux, Julien, John Stultz, Thomas Gleixner,
Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <20241219204208.3160-1-quic_philber@quicinc.com>
Support the new PTP_SYS_OFFSET_STAT ioctl. The virtio-rtc cross-timestamp
status information is aligned with the PTP_SYS_OFFSET_STAT output, so the
conversion is trivial.
Drop the getcrosststamp op implementation, for which the PTP clock core
will insert a getstattstamp wrapper.
Signed-off-by: Peter Hilber <quic_philber@quicinc.com>
---
drivers/virtio/Kconfig | 4 +-
drivers/virtio/virtio_rtc_driver.c | 122 ++++++++++++++++++++++++++-
drivers/virtio/virtio_rtc_internal.h | 3 +-
drivers/virtio/virtio_rtc_ptp.c | 25 +++---
4 files changed, 140 insertions(+), 14 deletions(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 6db5235a7693..7cb8b761eaa1 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -230,8 +230,8 @@ config VIRTIO_RTC_ARM
This enables Virtio RTC cross-timestamping using the Arm Generic Timer.
It only has an effect if the Virtio RTC device also supports this. The
cross-timestamp is available through the PTP clock driver precise
- cross-timestamp ioctl (PTP_SYS_OFFSET_PRECISE2 aka
- PTP_SYS_OFFSET_PRECISE).
+ cross-timestamp ioctls (PTP_SYS_OFFSET_PRECISE2 aka
+ PTP_SYS_OFFSET_PRECISE, PTP_SYS_OFFSET_STAT).
If unsure, say Y.
diff --git a/drivers/virtio/virtio_rtc_driver.c b/drivers/virtio/virtio_rtc_driver.c
index f8b890afc528..055aa1166519 100644
--- a/drivers/virtio/virtio_rtc_driver.c
+++ b/drivers/virtio/virtio_rtc_driver.c
@@ -13,6 +13,7 @@
#include <linux/device.h>
#include <linux/module.h>
#include <linux/pm.h>
+#include <linux/ptp_clock.h>
#include <uapi/linux/virtio_rtc.h>
@@ -617,16 +618,22 @@ int viortc_read(struct viortc_dev *viortc, u16 vio_clk_id, u64 *reading)
* @hw_counter: virtio_rtc HW counter type
* @reading: clock reading [ns]
* @cycles: HW counter cycles during clock reading
+ * @stat_extra: extra information, if non-NULL
*
* Context: Process context.
* Return: Zero on success, negative error code otherwise.
*/
int viortc_read_cross(struct viortc_dev *viortc, u16 vio_clk_id, u8 hw_counter,
- u64 *reading, u64 *cycles)
+ u64 *reading, u64 *cycles,
+ struct ptp_stat_extra *stat_extra)
{
VIORTC_DECLARE_MSG_HDL_ONSTACK(hdl, VIRTIO_RTC_REQ_READ_CROSS,
struct virtio_rtc_req_read_cross,
struct virtio_rtc_resp_read_cross);
+ struct ptp_clock_accuracy *accuracy;
+ u8 flags, leap, clock_status;
+ u32 smear_offset_nsec;
+ u16 tai_offset_sec;
int ret;
ret = VIORTC_MSG_INIT(hdl, viortc);
@@ -647,6 +654,119 @@ int viortc_read_cross(struct viortc_dev *viortc, u16 vio_clk_id, u8 hw_counter,
VIORTC_MSG_READ(hdl, clock_reading, reading);
VIORTC_MSG_READ(hdl, counter_cycles, cycles);
+ if (stat_extra) {
+ accuracy = &stat_extra->accuracy;
+
+ VIORTC_MSG_READ(hdl, perf.freq_esterror,
+ &accuracy->freq_esterror);
+ VIORTC_MSG_READ(hdl, perf.freq_maxerror,
+ &accuracy->freq_maxerror);
+ VIORTC_MSG_READ(hdl, perf.time_esterror,
+ &accuracy->time_esterror);
+ VIORTC_MSG_READ(hdl, perf.time_maxerror,
+ &accuracy->time_maxerror);
+
+ VIORTC_MSG_READ(hdl, perf.flags, &flags);
+
+ accuracy->flags = 0;
+
+ if (flags & VIRTIO_RTC_FLAG_FREQ_ESTERROR_VALID)
+ accuracy->flags |= PTP_CLOCK_FREQ_EST_VALID;
+
+ if (flags & VIRTIO_RTC_FLAG_FREQ_MAXERROR_VALID)
+ accuracy->flags |= PTP_CLOCK_FREQ_MAX_VALID;
+
+ if (flags & VIRTIO_RTC_FLAG_TIME_ESTERROR_VALID)
+ accuracy->flags |= PTP_CLOCK_TIME_EST_VALID;
+
+ if (flags & VIRTIO_RTC_FLAG_TIME_MAXERROR_VALID)
+ accuracy->flags |= PTP_CLOCK_TIME_MAX_VALID;
+
+ VIORTC_MSG_READ(hdl, perf.clock_status, &clock_status);
+
+ switch (clock_status) {
+ case VIRTIO_RTC_STATUS_INITIALIZING:
+ accuracy->clock_status = PTP_CLOCK_STATUS_INITIALIZING;
+ break;
+ case VIRTIO_RTC_STATUS_SYNCHRONIZED:
+ accuracy->clock_status = PTP_CLOCK_STATUS_SYNCHRONIZED;
+ break;
+ case VIRTIO_RTC_STATUS_FREERUNNING:
+ accuracy->clock_status = PTP_CLOCK_STATUS_FREERUNNING;
+ break;
+ case VIRTIO_RTC_STATUS_UNRELIABLE:
+ accuracy->clock_status = PTP_CLOCK_STATUS_UNRELIABLE;
+ break;
+ default:
+ accuracy->clock_status = PTP_CLOCK_STATUS_UNKNOWN;
+ break;
+ }
+
+ VIORTC_MSG_READ(hdl, leap_info.flags, &flags);
+
+ stat_extra->flags = 0;
+
+ if (flags & VIRTIO_RTC_FLAG_LEAP_VALID)
+ stat_extra->flags |= PTP_CLOCK_LEAP_VALID;
+
+ if (flags & VIRTIO_RTC_FLAG_TAI_OFFSET_VALID)
+ stat_extra->flags |= PTP_CLOCK_TAI_OFFSET_VALID;
+
+ if (flags & VIRTIO_RTC_FLAG_SMEAR_OFFSET_VALID)
+ stat_extra->flags |= PTP_CLOCK_SMEAR_OFFSET_VALID;
+
+ VIORTC_MSG_READ(hdl, leap_info.leap, &leap);
+
+ switch (leap) {
+ case VIRTIO_RTC_LEAP_NONE:
+ stat_extra->leap = PTP_LEAP_NONE;
+ break;
+ case VIRTIO_RTC_LEAP_PRE_POS:
+ stat_extra->leap = PTP_LEAP_PRE_POS;
+ break;
+ case VIRTIO_RTC_LEAP_PRE_NEG:
+ stat_extra->leap = PTP_LEAP_PRE_NEG;
+ break;
+ case VIRTIO_RTC_LEAP_POS:
+ stat_extra->leap = PTP_LEAP_POS;
+ break;
+ case VIRTIO_RTC_LEAP_POST_POS:
+ stat_extra->leap = PTP_LEAP_POST_POS;
+ break;
+ case VIRTIO_RTC_LEAP_POST_NEG:
+ stat_extra->leap = PTP_LEAP_POST_NEG;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_PRE_POS:
+ stat_extra->leap = PTP_LEAP_SMEAR_PRE_POS;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_PRE_NEG:
+ stat_extra->leap = PTP_LEAP_SMEAR_PRE_NEG;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_POS:
+ stat_extra->leap = PTP_LEAP_SMEAR_POS;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_NEG:
+ stat_extra->leap = PTP_LEAP_SMEAR_NEG;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_POST_POS:
+ stat_extra->leap = PTP_LEAP_SMEAR_POST_POS;
+ break;
+ case VIRTIO_RTC_LEAP_SMEAR_POST_NEG:
+ stat_extra->leap = PTP_LEAP_SMEAR_POST_NEG;
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_release;
+ }
+
+ VIORTC_MSG_READ(hdl, leap_info.tai_offset_sec, &tai_offset_sec);
+ stat_extra->tai_offset_sec = (s16)tai_offset_sec;
+
+ VIORTC_MSG_READ(hdl, leap_info.smear_offset_nsec,
+ &smear_offset_nsec);
+ stat_extra->smear_offset_nsec = (s32)smear_offset_nsec;
+ }
+
out_release:
viortc_msg_release(VIORTC_MSG(hdl));
diff --git a/drivers/virtio/virtio_rtc_internal.h b/drivers/virtio/virtio_rtc_internal.h
index e7f865259afd..ab998e033f07 100644
--- a/drivers/virtio/virtio_rtc_internal.h
+++ b/drivers/virtio/virtio_rtc_internal.h
@@ -20,7 +20,8 @@ struct viortc_dev;
int viortc_read(struct viortc_dev *viortc, u16 vio_clk_id, u64 *reading);
int viortc_read_cross(struct viortc_dev *viortc, u16 vio_clk_id, u8 hw_counter,
- u64 *reading, u64 *cycles);
+ u64 *reading, u64 *cycles,
+ struct ptp_stat_extra *stat_extra);
int viortc_cross_cap(struct viortc_dev *viortc, u16 vio_clk_id, u8 hw_counter,
bool *supported);
int viortc_read_alarm(struct viortc_dev *viortc, u16 vio_clk_id,
diff --git a/drivers/virtio/virtio_rtc_ptp.c b/drivers/virtio/virtio_rtc_ptp.c
index 09f5a9adf2e4..1a02ee3121d9 100644
--- a/drivers/virtio/virtio_rtc_ptp.c
+++ b/drivers/virtio/virtio_rtc_ptp.c
@@ -79,6 +79,7 @@ static int viortc_ptp_get_time_fn(ktime_t *device_time,
* @hw_counter: virtio_rtc HW counter type
* @cs_id: clocksource id corresponding to hw_counter
* @ctx: context for get_device_system_crosststamp()
+ * @stat_extra: extra information, if non-NULL
*
* Reads HW-specific crosststamp from device.
*
@@ -87,7 +88,8 @@ static int viortc_ptp_get_time_fn(ktime_t *device_time,
*/
static int viortc_ptp_do_xtstamp(struct viortc_ptp_clock *vio_ptp,
u8 hw_counter, enum clocksource_ids cs_id,
- struct viortc_ptp_cross_ctx *ctx)
+ struct viortc_ptp_cross_ctx *ctx,
+ struct ptp_stat_extra *stat_extra)
{
u64 ns;
u64 max_ns;
@@ -96,8 +98,8 @@ static int viortc_ptp_do_xtstamp(struct viortc_ptp_clock *vio_ptp,
ctx->system_counterval.cs_id = cs_id;
ret = viortc_read_cross(vio_ptp->viortc, vio_ptp->vio_clk_id,
- hw_counter, &ns,
- &ctx->system_counterval.cycles);
+ hw_counter, &ns, &ctx->system_counterval.cycles,
+ stat_extra);
if (ret)
return ret;
@@ -115,15 +117,17 @@ static int viortc_ptp_do_xtstamp(struct viortc_ptp_clock *vio_ptp,
*/
/**
- * viortc_ptp_getcrosststamp() - PTP clock getcrosststamp op
+ * viortc_ptp_getstattstamp() - PTP clock getcrosststamp with extras op
* @ptp: PTP clock info
* @xtstamp: crosststamp
+ * @stat_extra: extra information, if non-NULL
*
* Context: Process context.
* Return: Zero on success, negative error code otherwise.
*/
-static int viortc_ptp_getcrosststamp(struct ptp_clock_info *ptp,
- struct system_device_crosststamp *xtstamp)
+static int viortc_ptp_getstattstamp(struct ptp_clock_info *ptp,
+ struct system_device_crosststamp *xtstamp,
+ struct ptp_stat_extra *stat_extra)
{
struct viortc_ptp_clock *vio_ptp =
container_of(ptp, struct viortc_ptp_clock, ptp_info);
@@ -152,7 +156,8 @@ static int viortc_ptp_getcrosststamp(struct ptp_clock_info *ptp,
*
* So, get the actual cross-timestamp first.
*/
- ret = viortc_ptp_do_xtstamp(vio_ptp, hw_counter, cs_id, &ctx);
+ ret = viortc_ptp_do_xtstamp(vio_ptp, hw_counter, cs_id, &ctx,
+ stat_extra);
if (ret)
return ret;
@@ -225,7 +230,7 @@ static int viortc_ptp_enable(struct ptp_clock_info *ptp,
*
* The .name member will be set for individual virtio_rtc PTP clocks.
*
- * The .getcrosststamp member will be cleared for PTP clocks not supporting
+ * The .getstattstamp member will be cleared for PTP clocks not supporting
* crosststamp.
*/
static const struct ptp_clock_info viortc_ptp_info_template = {
@@ -236,7 +241,7 @@ static const struct ptp_clock_info viortc_ptp_info_template = {
.gettimex64 = viortc_ptp_gettimex64,
.settime64 = viortc_ptp_settime64,
.enable = viortc_ptp_enable,
- .getcrosststamp = viortc_ptp_getcrosststamp,
+ .getstattstamp = viortc_ptp_getstattstamp,
};
/**
@@ -329,7 +334,7 @@ struct viortc_ptp_clock *viortc_ptp_register(struct viortc_dev *viortc,
goto err_free_dev;
if (!vio_ptp->have_cross)
- vio_ptp->ptp_info.getcrosststamp = NULL;
+ vio_ptp->ptp_info.getstattstamp = NULL;
ptp_clock = ptp_clock_register(&vio_ptp->ptp_info, parent_dev);
if (IS_ERR(ptp_clock))
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Richard Cochran @ 2024-12-20 15:19 UTC (permalink / raw)
To: Peter Hilber
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <20241219204208.3160-2-quic_philber@quicinc.com>
On Thu, Dec 19, 2024 at 09:42:03PM +0100, Peter Hilber wrote:
> Ioctl PTP_SYS_OFFSET_PRECISE2 provides cross-timestamping of device time
> and system time. This can be used for virtualization where (virtualization)
> host and guest refer to the same clocksource. It may be preferred to
> indicate UTC time, rather than TAI. It is then useful to indicate when and
> how the host processes UTC leap seconds (stepping or smearing on leap
> seconds),
If the VM host provides TAI, then the guest may freely derive UTC and
leap seconds on its own. Whether to smear leap seconds or not is
properly an administrative configuration choice in the VM guest.
Leap seconds are scheduled to be deleted in 2036. If, between now and
then, another one occurs, it will be global event, not determined by a
VM host. The way you find out about leap seconds is through
networking using the NTP (or even just downloading the published list
once in a while). VM clients typically have networking, and so
they can learn about a leap second all by themselves.
> in particular if the guest is not guaranteed to have an
> up-to-date Time Zone Database or similar.
Time zones are not connected to leap seconds.
> Also, a host may have a notion of how accurate its clock is w.r.t. the
> hosts' reference clocks.
I'm opposed to having device drivers try to claim any kind of clock
quality. All of the clock control, servos, statistics, etc, are done
in user space, and so only user space software can generate meaningful
clock quality metrics. Putting some kind of hand wavy values into
kernel drivers is just plain wrong IMO.
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Peter Hilber @ 2024-12-23 18:13 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <Z2WLGHRdlsRpT6BL@hoboy.vegasvil.org>
On Fri, Dec 20, 2024 at 07:19:52AM -0800, Richard Cochran wrote:
> On Thu, Dec 19, 2024 at 09:42:03PM +0100, Peter Hilber wrote:
> > Ioctl PTP_SYS_OFFSET_PRECISE2 provides cross-timestamping of device time
> > and system time. This can be used for virtualization where (virtualization)
> > host and guest refer to the same clocksource. It may be preferred to
> > indicate UTC time, rather than TAI. It is then useful to indicate when and
> > how the host processes UTC leap seconds (stepping or smearing on leap
> > seconds),
>
> If the VM host provides TAI, then the guest may freely derive UTC and
> leap seconds on its own. Whether to smear leap seconds or not is
> properly an administrative configuration choice in the VM guest.
>
The precise synchronization of the VM guest with its immediate
environment can also be important; a VM guest may depend the decision
about leap second smearing on its environment. Also, the administrative
configuration choice may change over the lifetime of a system.
> Leap seconds are scheduled to be deleted in 2036. If, between now and
> then, another one occurs, it will be global event, not determined by a
> VM host. The way you find out about leap seconds is through
> networking using the NTP (or even just downloading the published list
> once in a while). VM clients typically have networking, and so
> they can learn about a leap second all by themselves.
>
The intent is to also support (embedded) VM clients which are themselves
not necessarily internetworked, which do not get a lot of maintenance,
and which are not guaranteed to get an update within the typically less
than 6 months between leap second announcement and occurrence. I will
document this in the patch message.
> > in particular if the guest is not guaranteed to have an
> > up-to-date Time Zone Database or similar.
>
> Time zones are not connected to leap seconds.
>
The IANA Time Zone Database includes a leap seconds list.
> > Also, a host may have a notion of how accurate its clock is w.r.t. the
> > hosts' reference clocks.
>
> I'm opposed to having device drivers try to claim any kind of clock
> quality. All of the clock control, servos, statistics, etc, are done
> in user space, and so only user space software can generate meaningful
> clock quality metrics. Putting some kind of hand wavy values into
> kernel drivers is just plain wrong IMO.
I agree that a device driver should not determine clock quality metrics.
The intent is that the driver forwards metrics, if such are advertised
by the device. These metrics should describe the accuracy etc. of the
device itself. The patch message should document this more clearly. The
metrics can be determined e.g. by virtualization host user space
software. The device driver would just expose the device metrics to user
space.
Thanks for the feedback,
Peter
^ permalink raw reply
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Richard Cochran @ 2024-12-26 0:42 UTC (permalink / raw)
To: Peter Hilber
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <wcxdbqhoe4cppukyy5rvkq5am4ht6wk5u6d6g2k2swqhidjw7i@6nar5vuusm35>
On Mon, Dec 23, 2024 at 07:13:46PM +0100, Peter Hilber wrote:
> The precise synchronization of the VM guest with its immediate
> environment can also be important; a VM guest may depend the decision
> about leap second smearing on its environment.
I thought that the whole point of using a VM is to isolate the guests
from each other and the host. What you describe is a promiscuous
coupling between guest and host, and the kernel shouldn't be in
the business of supporting such behavior.
> Also, the administrative
> configuration choice may change over the lifetime of a system.
Right, which is why we should keep those choices out of kernel space.
Kernel provides mechanism, not policy.
> The intent is to also support (embedded) VM clients which are themselves
> not necessarily internetworked, which do not get a lot of maintenance,
> and which are not guaranteed to get an update within the typically less
> than 6 months between leap second announcement and occurrence.
Again, I don't think the kernel should be the solution to guests that
lack networking. Instead, the place to fix the problem is at the
root, namely in the guests.
> I agree that a device driver should not determine clock quality metrics.
> The intent is that the driver forwards metrics, if such are advertised
> by the device. These metrics should describe the accuracy etc. of the
> device itself.
Overall, I don't trust devices to tell the truth about their
qualities. But putting that aside, we would need to see some kind of
commonality in hardware implementation to advertise their metrics.
However, AFAICT there is no such industry practice on the market.
> The patch message should document this more clearly. The
> metrics can be determined e.g. by virtualization host user space
> software. The device driver would just expose the device metrics to user
> space.
Again, host user space shouldn't misuse the kernel to share random
metrics with guest user space. Isn't there another way to share such
info from host to guest?
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Peter Hilber @ 2025-01-02 16:11 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <Z2ymZuiFqY8mxihJ@hoboy.vegasvil.org>
On Wed, Dec 25, 2024 at 04:42:14PM -0800, Richard Cochran wrote:
> On Mon, Dec 23, 2024 at 07:13:46PM +0100, Peter Hilber wrote:
>
> > The precise synchronization of the VM guest with its immediate
> > environment can also be important; a VM guest may depend the decision
> > about leap second smearing on its environment.
>
> I thought that the whole point of using a VM is to isolate the guests
> from each other and the host. What you describe is a promiscuous
> coupling between guest and host, and the kernel shouldn't be in
> the business of supporting such behavior.
>
Why? There is already ptp_kvm etc. in the kernel.
Would it be more acceptable to just announce leap seconds, but not
whether to smear?
> > Also, the administrative
> > configuration choice may change over the lifetime of a system.
>
> Right, which is why we should keep those choices out of kernel space.
> Kernel provides mechanism, not policy.
>
As discussed, the policy would be forwarded, not determined, by the
kernel.
If the policy would be forwarded via NTP (by the NTP server smearing or
not smearing leap seconds), this would have the following disadvantages:
- need to use an NTP server just for announcing leap seconds
- redundancy of serving time both via NTP and via PTP clocks (for better
precision)
- no awareness about leap seconds in case of smearing.
> > The intent is to also support (embedded) VM clients which are themselves
> > not necessarily internetworked, which do not get a lot of maintenance,
> > and which are not guaranteed to get an update within the typically less
> > than 6 months between leap second announcement and occurrence.
>
> Again, I don't think the kernel should be the solution to guests that
> lack networking. Instead, the place to fix the problem is at the
> root, namely in the guests.
>
I do not understand. Is the point that guests should decide through
another channel about leap second smearing?
> > I agree that a device driver should not determine clock quality metrics.
> > The intent is that the driver forwards metrics, if such are advertised
> > by the device. These metrics should describe the accuracy etc. of the
> > device itself.
>
> Overall, I don't trust devices to tell the truth about their
> qualities. But putting that aside, we would need to see some kind of
> commonality in hardware implementation to advertise their metrics.
> However, AFAICT there is no such industry practice on the market.
>
I hope there will be some feedback from third parties (at least related
to virtualization).
> > The patch message should document this more clearly. The
> > metrics can be determined e.g. by virtualization host user space
> > software. The device driver would just expose the device metrics to user
> > space.
>
> Again, host user space shouldn't misuse the kernel to share random
> metrics with guest user space. Isn't there another way to share such
> info from host to guest?
>
For sure. But the aim of this proposal is to have an interoperable time
synchronization solution for VMs through a Virtio device. So the idea is
to include metrics, if a consensus on their usefulness can be reached.
AFAIU it is difficult to bypass the kernel for Virtio devices.
Thanks for the discussion,
Peter
^ permalink raw reply
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Richard Cochran @ 2025-01-02 16:21 UTC (permalink / raw)
To: Peter Hilber
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <a352mltlizneonxazn4bffydn57fyudrc3zougii2rnatg3jga@3yagssaob5sb>
On Thu, Jan 02, 2025 at 05:11:01PM +0100, Peter Hilber wrote:
> Would it be more acceptable to just announce leap seconds, but not
> whether to smear?
Up until now, leap second announcements were handled in user space,
and the kernel played no role.
> I do not understand. Is the point that guests should decide through
> another channel about leap second smearing?
Yes, that would make more sense to me.
> I hope there will be some feedback from third parties (at least related
> to virtualization).
+1
I'm no VM expert, but I'd like to avoid tacking things onto the kernel
PTP layer, unless there is a really strong justification.
> For sure. But the aim of this proposal is to have an interoperable time
> synchronization solution for VMs through a Virtio device. So the idea is
> to include metrics, if a consensus on their usefulness can be reached.
> AFAIU it is difficult to bypass the kernel for Virtio devices.
Providing clock metrics only makes sense when there is some choice to
be made based on those metrics. If the "limited" VM guests don't even
have networking, then they have no choice but to accept the time from
the VM host, right? In which case, the metrics do not provide any
benefit to the guest.
Or what am I missing?
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH 1/2] ptp: add PTP_SYS_OFFSET_STAT for xtstamping with status
From: Peter Hilber @ 2025-01-06 17:46 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel, virtualization, virtio-dev, netdev, Trilok Soni,
Srivatsa Vaddagiri, David S. Miller, Eugenio Pérez,
Michael S. Tsirkin, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Jason Wang, Paolo Abeni, Shuah Khan, Xuan Zhuo, linux-kselftest,
linux-api, David Woodhouse, Ridoux, Julien, John Stultz,
Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen
In-Reply-To: <Z3a892mBOSRl6BlN@hoboy.vegasvil.org>
On Thu, Jan 02, 2025 at 08:21:11AM -0800, Richard Cochran wrote:
> On Thu, Jan 02, 2025 at 05:11:01PM +0100, Peter Hilber wrote:
> > For sure. But the aim of this proposal is to have an interoperable time
> > synchronization solution for VMs through a Virtio device. So the idea is
> > to include metrics, if a consensus on their usefulness can be reached.
> > AFAIU it is difficult to bypass the kernel for Virtio devices.
>
> Providing clock metrics only makes sense when there is some choice to
> be made based on those metrics. If the "limited" VM guests don't even
> have networking, then they have no choice but to accept the time from
> the VM host, right? In which case, the metrics do not provide any
> benefit to the guest.
>
> Or what am I missing?
The proposal is not limited to guests with no networking.
Guests *might* not have internet access (might not be "internetworked").
Even then, guests can still interact with the environment (so metrics
can be important) and might decide to reduce features when the metrics
are too bad.
Thanks for the comments,
Peter
^ permalink raw reply
* Re: [PATCH v6 2/5] pidfd: add PIDFD_SELF_* sentinels to refer to own thread/process
From: Shakeel Butt @ 2025-01-06 21:03 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Christian Brauner, Oleg Nesterov, Christian Brauner, Shuah Khan,
Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <fbcea328-9545-4f3e-9f99-2e2057ce32df@lucifer.local>
On Mon, Dec 02, 2024 at 10:52:13AM +0000, Lorenzo Stoakes wrote:
> On Fri, Nov 08, 2024 at 02:28:14PM +0000, Lorenzo Stoakes wrote:
> > On Wed, Oct 30, 2024 at 04:37:37PM +0000, Lorenzo Stoakes wrote:
> > > On Mon, Oct 28, 2024 at 04:06:07PM +0000, Lorenzo Stoakes wrote:
> > > > I guess I'll try to adapt that and respin a v7 when I get a chance.
> > >
> > > Hm looking at this draft patch, it seems like a total rework of pidfd's
> > > across the board right (now all pidfd's will need to be converted to
> > > pid_fd)? Correct me if I'm wrong.
> > >
> > > If only for the signal case, it seems like overkill to define a whole
> > > pid_fd and to use this CLASS() wrapper just for this one instance.
> > >
> > > If the intent is to convert _all_ pidfd's to use this type, it feels really
> > > out of scope for this series and I think we'd probably instead want to go
> > > off and do that as a separate series and put this on hold until that is
> > > done.
> > >
> > > If instead you mean that we ought to do something like this just for the
> > > signal case, it feels like it'd be quite a bit of extra abstraction just
> > > used in this one case but nowhere else, I think if you did an abstraction
> > > like this it would _have_ to be across the board right?
> > >
> > > I agree that the issue is with this one signal case that pins only the fd
> > > (rather than this pid) where this 'pinning' doesn't _necessary_ mess around
> > > with reference counts.
> > >
> > > So we definitely must address this, but the issue you had with the first
> > > approach was that I think (correct me if I'm wrong) I was passing a pointer
> > > to a struct fd which is not permitted right?
> > >
> > > Could we pass the struct fd by value to avoid this? I think we'd have to
> > > unfortunately special-case this and probably duplicate some code which is a
> > > pity as I liked the idea of abstracting everything to one place, but we can
> > > obviously do that.
> > >
> > > So I guess to TL;DR it, the options are:
> > >
> > > 1. Implement pid_fd everywhere, in which case I will leave off on
> > > this series and I guess, if I have time I could look at trying to
> > > implement that or perhaps you'd prefer to?
> > >
> > > 2. We are good for the sake of this series to special-case a pidfd_to_pid()
> > > implementation (used only by the pidfd_send_signal() syscall)
> > >
> > > 3. Something else, or I am misunderstanding your point :)
> > >
> > > Let me know how you want me to proceed on this as we're at v6 already and I
> > > want to be _really_ sure I'm doing what you want here.
> > >
> > > Thanks!
> >
> > Hi Christian,
> >
> > Just a gentle nudge on this - as I need some guidance in order to know how
> > to move the series forwards.
> >
> > Obviously no rush if your workload is high at the moment as this is pretty
> > low priority, but just in case you missed it :)
> >
> > Thanks, Lorenzo
>
> Hi Christian,
>
> Just a ping on this now we're past the merge window and it's been over a
> month.
>
> It'd be good to at least get a polite ack to indicate you're aware even if
> you don't have the time to respond right now.
>
> If you'd prefer this series not to go ahead just let me know, but
> unfortunately I really require your input to know how to move forward
> otherwise I risk doing work that you might then reject.
>
Hey Lorenzo & Christian, what's the latest here? I see Christian has
code suggestions at [1] which just needs to be addressed. Any thing
else? I am hoping we can get this merged in the coming open window.
[1] https://lore.kernel.org/linux-mm/20241202-wahrnehmen-mitten-e330cbd1eaf0@brauner/
^ permalink raw reply
* Re: [PATCH v6 2/5] pidfd: add PIDFD_SELF_* sentinels to refer to own thread/process
From: Lorenzo Stoakes @ 2025-01-07 8:32 UTC (permalink / raw)
To: Shakeel Butt
Cc: Christian Brauner, Oleg Nesterov, Christian Brauner, Shuah Khan,
Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <wvwahrb5yxd4dzfy6an7dpefiswtbc5uutckltbtx5p7hv4pxg@4n6tds6cmjx3>
On Mon, Jan 06, 2025 at 01:03:31PM -0800, Shakeel Butt wrote:
> Hey Lorenzo & Christian, what's the latest here? I see Christian has
> code suggestions at [1] which just needs to be addressed. Any thing
> else? I am hoping we can get this merged in the coming open window.
This is on my radar don't worry :) I will get to it as soon as I can.
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v7 00/30] NT synchronization primitive driver
From: Greg Kroah-Hartman @ 2025-01-07 17:06 UTC (permalink / raw)
To: Elizabeth Figura
Cc: Arnd Bergmann, Jonathan Corbet, Shuah Khan, 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: <20241213193511.457338-1-zfigura@codeweavers.com>
On Fri, Dec 13, 2024 at 01:34:41PM -0600, Elizabeth Figura wrote:
> 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
>
Given a lack of complaints, I've now applied this to my testing tree.
Thanks for sticking with it!
greg k-h
^ permalink raw reply
* Re: [PATCH v7 00/30] NT synchronization primitive driver
From: Peter Zijlstra @ 2025-01-07 19:42 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Elizabeth Figura, Arnd Bergmann, 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: <2025010738-amicably-art-f746@gregkh>
On Tue, Jan 07, 2025 at 06:06:03PM +0100, Greg Kroah-Hartman wrote:
> Given a lack of complaints, I've now applied this to my testing tree.
> Thanks for sticking with it!
Right, so I acked v6, which wasn't preserved. The v7 changes are minor
and seem fine (IIRc I even suggested them some very very long time ago).
^ permalink raw reply
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