Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
From: kernel test robot @ 2025-01-09  2:21 UTC (permalink / raw)
  To: Dmitry V. Levin, Oleg Nesterov
  Cc: llvm, oe-kbuild-all, Eugene Syromyatnikov, Mike Frysinger,
	Renzo Davoli, Davide Berardi, strace-devel, linux-kernel,
	linux-api
In-Reply-To: <20250107230456.GE30633@strace.io>

Hi Dmitry,

kernel test robot noticed the following build errors:

[auto build test ERROR on openrisc/for-next]
[also build test ERROR on powerpc/next powerpc/fixes s390/features uml/next jcmvbkbc-xtensa/xtensa-for-next arnd-asm-generic/master vgupta-arc/for-curr arm64/for-next/core linus/master uml/fixes tip/x86/core vgupta-arc/for-next v6.13-rc6 next-20250108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-V-Levin/Revert-arch-remove-unused-function-syscall_set_arguments/20250108-070658
base:   https://github.com/openrisc/linux.git for-next
patch link:    https://lore.kernel.org/r/20250107230456.GE30633%40strace.io
patch subject: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
config: hexagon-randconfig-001-20250109 (https://download.01.org/0day-ci/archive/20250109/202501090954.gYTxI9sY-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250109/202501090954.gYTxI9sY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501090954.gYTxI9sY-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/ptrace.c:1053:3: error: implicit declaration of function 'syscall_set_return_value' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                   syscall_set_return_value(child, regs, -ENOSYS, 0);
                   ^
   kernel/ptrace.c:1053:3: note: did you mean 'syscall_get_return_value'?
   arch/hexagon/include/asm/syscall.h:56:20: note: 'syscall_get_return_value' declared here
   static inline long syscall_get_return_value(struct task_struct *task,
                      ^
   kernel/ptrace.c:1075:3: error: implicit declaration of function 'syscall_set_return_value' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                   syscall_set_return_value(child, regs, info->exit.rval, 0);
                   ^
   2 errors generated.


vim +/syscall_set_return_value +1053 kernel/ptrace.c

  1021	
  1022	static unsigned long
  1023	ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
  1024				      struct ptrace_syscall_info *info)
  1025	{
  1026		unsigned long args[ARRAY_SIZE(info->entry.args)];
  1027		int nr = info->entry.nr;
  1028		int i;
  1029	
  1030		if (nr != info->entry.nr)
  1031			return -ERANGE;
  1032	
  1033		for (i = 0; i < ARRAY_SIZE(args); i++) {
  1034			args[i] = info->entry.args[i];
  1035			if (args[i] != info->entry.args[i])
  1036				return -ERANGE;
  1037		}
  1038	
  1039		syscall_set_nr(child, regs, nr);
  1040		syscall_set_arguments(child, regs, args);
  1041		if (nr == -1) {
  1042			/*
  1043			 * When the syscall number is set to -1, the syscall will be
  1044			 * skipped.  In this case also set the syscall return value to
  1045			 * -ENOSYS, otherwise on some architectures the corresponding
  1046			 * struct pt_regs field will remain unchanged.
  1047			 *
  1048			 * Note that on some architectures syscall_set_return_value()
  1049			 * modifies one of the struct pt_regs fields also modified by
  1050			 * syscall_set_arguments(), so the former should be called
  1051			 * after the latter.
  1052			 */
> 1053			syscall_set_return_value(child, regs, -ENOSYS, 0);
  1054		}
  1055	
  1056		return 0;
  1057	}
  1058	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
From: kernel test robot @ 2025-01-09  1:37 UTC (permalink / raw)
  To: Dmitry V. Levin, Oleg Nesterov
  Cc: llvm, oe-kbuild-all, Eugene Syromyatnikov, Mike Frysinger,
	Renzo Davoli, Davide Berardi, strace-devel, linux-kernel,
	linux-api
In-Reply-To: <20250107230456.GE30633@strace.io>

Hi Dmitry,

kernel test robot noticed the following build errors:

[auto build test ERROR on openrisc/for-next]
[also build test ERROR on powerpc/next powerpc/fixes s390/features uml/next jcmvbkbc-xtensa/xtensa-for-next arnd-asm-generic/master vgupta-arc/for-curr arm64/for-next/core linus/master uml/fixes tip/x86/core vgupta-arc/for-next v6.13-rc6 next-20250108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-V-Levin/Revert-arch-remove-unused-function-syscall_set_arguments/20250108-070658
base:   https://github.com/openrisc/linux.git for-next
patch link:    https://lore.kernel.org/r/20250107230456.GE30633%40strace.io
patch subject: [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
config: hexagon-allnoconfig (https://download.01.org/0day-ci/archive/20250109/202501090919.TiLTOhaq-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 096551537b2a747a3387726ca618ceeb3950e9bc)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250109/202501090919.TiLTOhaq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501090919.TiLTOhaq-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/ptrace.c:1053:3: error: call to undeclared function 'syscall_set_return_value'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1053 |                 syscall_set_return_value(child, regs, -ENOSYS, 0);
         |                 ^
   kernel/ptrace.c:1053:3: note: did you mean 'syscall_get_return_value'?
   arch/hexagon/include/asm/syscall.h:56:20: note: 'syscall_get_return_value' declared here
      56 | static inline long syscall_get_return_value(struct task_struct *task,
         |                    ^
   kernel/ptrace.c:1075:3: error: call to undeclared function 'syscall_set_return_value'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1075 |                 syscall_set_return_value(child, regs, info->exit.rval, 0);
         |                 ^
   2 errors generated.


vim +/syscall_set_return_value +1053 kernel/ptrace.c

  1021	
  1022	static unsigned long
  1023	ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
  1024				      struct ptrace_syscall_info *info)
  1025	{
  1026		unsigned long args[ARRAY_SIZE(info->entry.args)];
  1027		int nr = info->entry.nr;
  1028		int i;
  1029	
  1030		if (nr != info->entry.nr)
  1031			return -ERANGE;
  1032	
  1033		for (i = 0; i < ARRAY_SIZE(args); i++) {
  1034			args[i] = info->entry.args[i];
  1035			if (args[i] != info->entry.args[i])
  1036				return -ERANGE;
  1037		}
  1038	
  1039		syscall_set_nr(child, regs, nr);
  1040		syscall_set_arguments(child, regs, args);
  1041		if (nr == -1) {
  1042			/*
  1043			 * When the syscall number is set to -1, the syscall will be
  1044			 * skipped.  In this case also set the syscall return value to
  1045			 * -ENOSYS, otherwise on some architectures the corresponding
  1046			 * struct pt_regs field will remain unchanged.
  1047			 *
  1048			 * Note that on some architectures syscall_set_return_value()
  1049			 * modifies one of the struct pt_regs fields also modified by
  1050			 * syscall_set_arguments(), so the former should be called
  1051			 * after the latter.
  1052			 */
> 1053			syscall_set_return_value(child, regs, -ENOSYS, 0);
  1054		}
  1055	
  1056		return 0;
  1057	}
  1058	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [PATCH 5/6] ptrace: introduce PTRACE_SET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2025-01-07 23:04 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Eugene Syromyatnikov, Mike Frysinger, Renzo Davoli,
	Davide Berardi, strace-devel, linux-kernel, linux-api
In-Reply-To: <20250107230153.GA30560@strace.io>

PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
system calls the tracee is blocked in.

This API allows ptracers to obtain and modify system call details
in a straightforward and architecture-agnostic way.

Current implementation supports changing only those bits of system call
information that are used by strace, namely, syscall number, syscall
arguments, and syscall return value.

Support of changing additional details returned by PTRACE_GET_SYSCALL_INFO,
such as instruction pointer and stack pointer, could be added later
if needed, by re-using struct ptrace_syscall_info.reserved to specify
the additional details that should be set.  Currently, the reserved
field of struct ptrace_syscall_info must be initialized with zeroes;
arch, instruction_pointer, and stack_pointer fields are ignored.

PTRACE_SET_SYSCALL_INFO currently supports only PTRACE_SYSCALL_INFO_ENTRY,
PTRACE_SYSCALL_INFO_EXIT, and PTRACE_SYSCALL_INFO_SECCOMP operations.
Other operations could be added later if needed.

Ideally, PTRACE_SET_SYSCALL_INFO should have been introduced along with
PTRACE_GET_SYSCALL_INFO, but it didn't happen.  The last straw that
convinced me to implement PTRACE_SET_SYSCALL_INFO was apparent failure
to provide an API of changing the first system call argument on riscv
architecture.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
...
PTRACE_SET_SYSCALL_INFO
       Modify information about the system call that caused the stop.
       The "data" argument is a pointer to struct ptrace_syscall_info
       that specifies the system call information to be set.
       The "addr" argument should be set to sizeof(struct ptrace_syscall_info)).

Link: https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/
Signed-off-by: Dmitry V. Levin <ldv@strace.io>
---
 include/linux/ptrace.h      |   3 ++
 include/uapi/linux/ptrace.h |   3 +-
 kernel/ptrace.c             | 102 ++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 90507d4afcd6..c8dbf1e498bf 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -17,6 +17,9 @@ struct syscall_info {
 	struct seccomp_data	data;
 };
 
+/* sizeof() the first published struct ptrace_syscall_info */
+#define PTRACE_SYSCALL_INFO_SIZE_VER0	84
+
 extern int ptrace_access_vm(struct task_struct *tsk, unsigned long addr,
 			    void *buf, int len, unsigned int gup_flags);
 
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 72c038fc71d0..231b8bf7eeff 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -74,6 +74,7 @@ struct seccomp_metadata {
 };
 
 #define PTRACE_GET_SYSCALL_INFO		0x420e
+#define PTRACE_SET_SYSCALL_INFO		0x4212
 #define PTRACE_SYSCALL_INFO_NONE	0
 #define PTRACE_SYSCALL_INFO_ENTRY	1
 #define PTRACE_SYSCALL_INFO_EXIT	2
@@ -81,7 +82,7 @@ struct seccomp_metadata {
 
 struct ptrace_syscall_info {
 	__u8 op;	/* PTRACE_SYSCALL_INFO_* */
-	__u8 pad[3];
+	__u8 reserved[3];
 	__u32 arch;
 	__u64 instruction_pointer;
 	__u64 stack_pointer;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index e7e0003cc8e0..52377653743d 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1018,6 +1018,104 @@ ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
 	write_size = min(actual_size, user_size);
 	return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
 }
+
+static unsigned long
+ptrace_set_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+			      struct ptrace_syscall_info *info)
+{
+	unsigned long args[ARRAY_SIZE(info->entry.args)];
+	int nr = info->entry.nr;
+	int i;
+
+	if (nr != info->entry.nr)
+		return -ERANGE;
+
+	for (i = 0; i < ARRAY_SIZE(args); i++) {
+		args[i] = info->entry.args[i];
+		if (args[i] != info->entry.args[i])
+			return -ERANGE;
+	}
+
+	syscall_set_nr(child, regs, nr);
+	syscall_set_arguments(child, regs, args);
+	if (nr == -1) {
+		/*
+		 * When the syscall number is set to -1, the syscall will be
+		 * skipped.  In this case also set the syscall return value to
+		 * -ENOSYS, otherwise on some architectures the corresponding
+		 * struct pt_regs field will remain unchanged.
+		 *
+		 * Note that on some architectures syscall_set_return_value()
+		 * modifies one of the struct pt_regs fields also modified by
+		 * syscall_set_arguments(), so the former should be called
+		 * after the latter.
+		 */
+		syscall_set_return_value(child, regs, -ENOSYS, 0);
+	}
+
+	return 0;
+}
+
+static unsigned long
+ptrace_set_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+				struct ptrace_syscall_info *info)
+{
+	/*
+	 * info->entry is currently a subset of info->seccomp,
+	 * info->seccomp.ret_data is currently ignored.
+	 */
+	return ptrace_set_syscall_info_entry(child, regs, info);
+}
+
+static unsigned long
+ptrace_set_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+			     struct ptrace_syscall_info *info)
+{
+	if (info->exit.is_error)
+		syscall_set_return_value(child, regs, info->exit.rval, 0);
+	else
+		syscall_set_return_value(child, regs, 0, info->exit.rval);
+
+	return 0;
+}
+
+static int
+ptrace_set_syscall_info(struct task_struct *child, unsigned long user_size,
+			void __user *datavp)
+{
+	struct pt_regs *regs = task_pt_regs(child);
+	struct ptrace_syscall_info info;
+	int error;
+
+	BUILD_BUG_ON(sizeof(struct ptrace_syscall_info) < PTRACE_SYSCALL_INFO_SIZE_VER0);
+
+	if (user_size < PTRACE_SYSCALL_INFO_SIZE_VER0 || user_size > PAGE_SIZE)
+		return -EINVAL;
+
+	error = copy_struct_from_user(&info, sizeof(info), datavp, user_size);
+	if (error)
+		return error;
+
+	/* Reserved for future use. */
+	if (memchr_inv(info.reserved, 0, sizeof(info.reserved)))
+		return -EINVAL;
+
+	/* Changing the type of the system call stop is not supported. */
+	if (ptrace_get_syscall_info_op(child) != info.op)
+		return -EINVAL;
+
+	switch (info.op) {
+	case PTRACE_SYSCALL_INFO_ENTRY:
+		return ptrace_set_syscall_info_entry(child, regs, &info);
+	case PTRACE_SYSCALL_INFO_EXIT:
+		return ptrace_set_syscall_info_exit(child, regs, &info);
+	case PTRACE_SYSCALL_INFO_SECCOMP:
+		return ptrace_set_syscall_info_seccomp(child, regs, &info);
+	default:
+		/* Other types of system call stops are not supported. */
+		return -EINVAL;
+	}
+}
 #endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
 
 int ptrace_request(struct task_struct *child, long request,
@@ -1236,6 +1334,10 @@ int ptrace_request(struct task_struct *child, long request,
 	case PTRACE_GET_SYSCALL_INFO:
 		ret = ptrace_get_syscall_info(child, addr, datavp);
 		break;
+
+	case PTRACE_SET_SYSCALL_INFO:
+		ret = ptrace_set_syscall_info(child, addr, datavp);
+		break;
 #endif
 
 	case PTRACE_SECCOMP_GET_FILTER:

-- 
ldv

^ permalink raw reply related

* Re: [PATCH v7 00/30] NT synchronization primitive driver
From: Elizabeth Figura @ 2025-01-07 21:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Zijlstra
  Cc: 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: <20250107194241.GC28303@noisy.programming.kicks-ass.net>

On Tuesday, 7 January 2025 13:42:41 CST Peter Zijlstra wrote:
> 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).

I wasn't sure it was fine to preserve the ack across changes (even if they were the ones you suggested) and decided to err on the safe side.

Thank you for the review!



^ 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

* 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 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 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: [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: [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-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 @ 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 @ 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-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

* [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

* [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 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

* 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

* 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 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 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-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 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 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

* [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 = &current->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 = &current->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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox