Introduce struct __kernel_timex
Subsystems (11)
v3 cover Introduce struct __kernel_timex
The series introduces struct __kernel_timex as a substitute for the non y2038 safe struct timex.
The series is based on the original series posted by Arnd Bergmann in [1].
The overview of the series is as below:
1. Prepare for the compat timex interfaces to be used unconditionally.
2. Introduce struct __kernel_timex.
3. Use struct __kernel_timex in place of struct timex.
4. Switch syscalls to use struct __kernel_timex.
[1] https://sourceware.org/ml/libc-alpha/2015-05/msg00070.html
Changes since v2:
* Use only generic compat.h for riscv
Changes since v1:
* Fix riscv asm/compat.h to pick up generic compat types
Deepa Dinamani (7):
arm64: Make basic compat_* types always available
sparc: Make thread_info.h available directly
riscv: Delete asm/compat.h
timex: prepare compat helpers for y2038 changes
time: Add struct __kernel_timex
timex: use __kernel_timex internally
timex: change syscalls to use struct __kernel_timex
arch/alpha/kernel/osf_sys.c | 2 +- arch/arm64/include/asm/compat.h | 22 ++++----- arch/riscv/include/asm/Kbuild | 1 + arch/riscv/include/asm/compat.h | 29 ------------ arch/sparc/include/asm/compat.h | 2 + drivers/ptp/ptp_clock.c | 2 +- include/asm-generic/compat.h | 8 +++- include/linux/compat.h | 33 -------------- include/linux/compat_time.h | 34 ++++++++++++++ include/linux/posix-clock.h | 2 +- include/linux/syscalls.h | 5 +-- include/linux/timex.h | 9 +++- include/uapi/linux/timex.h | 41 +++++++++++++++++ kernel/compat.c | 63 -------------------------- kernel/time/ntp.c | 12 ++--- kernel/time/ntp_internal.h | 2 +- kernel/time/posix-clock.c | 2 +- kernel/time/posix-timers.c | 14 ++---- kernel/time/posix-timers.h | 2 +- kernel/time/time.c | 80 ++++++++++++++++++++++++++++++--- kernel/time/timekeeping.c | 4 +- 21 files changed, 199 insertions(+), 170 deletions(-) delete mode 100644 arch/riscv/include/asm/compat.h
base-commit: 526674536360a4c508e84f67314c2028e45e1bf2
Signature · 14 lines
2.17.1 Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] _______________________________________________ Y2038 mailing list [email protected] https://lists.linaro.org/mailman/listinfo/y2038
PATCH v3 1/7 arm64: Make basic compat_* types always available
As we repurpose more compat syscalls to be used in non CONFIG_COMPAT usecases as part of solving y2038, we need to make these basic types available unconditionally.
--- arch/arm64/include/asm/compat.h | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
arch/arm64/include/asm/compat.h +12 -10
@@ -16,21 +16,14 @@#ifndef __ASM_COMPAT_H#define __ASM_COMPAT_H#ifdef __KERNEL__ -#ifdef CONFIG_COMPAT -/* -* Architecture specific compatibility types -*/ #include <linux/types.h> #include <linux/sched.h> #include <linux/sched/task_stack.h> -#define COMPAT_USER_HZ 100 -#ifdef __AARCH64EB__ -#define COMPAT_UTS_MACHINE "armv8b\0\0" -#else -#define COMPAT_UTS_MACHINE "armv8l\0\0" -#endif +/* +* Architecture specific compatibility types +*/ typedef u32 compat_size_t ;typedef s32 compat_ssize_t ;
@@ -65,6 +58,15 @@ typedef u32 compat_ulong_t;typedef u64 compat_u64 ;typedef u32 compat_uptr_t ; +#ifdef CONFIG_COMPAT + +#define COMPAT_USER_HZ 100 +#ifdef __AARCH64EB__ +#define COMPAT_UTS_MACHINE "armv8b\0\0" +#else +#define COMPAT_UTS_MACHINE "armv8l\0\0" +#endif + struct compat_stat { #ifdef __AARCH64EB__ short st_dev;
Signature · 1 line
2.17.1
Re: [PATCH v3 1/7] arm64: Make basic compat_* types always available
Quoted 3 lines · Deepa Dinamani
As we repurpose more compat syscalls to be used in non
CONFIG_COMPATusecases as part of solving y2038, we need to make these basic types available unconditionally.
NAK. Compat code is for foreign ABIs. If this code is going to be used elsewhere it should not be named compat_ and not be in compat.h.
PATCH v3 2/7 sparc: Make thread_info.h available directly
In reply to Deepa Dinamanilinux/compat.h was including this indirectly forasm/compat.h dependencies.
--- arch/sparc/include/asm/compat.h | 2 ++ 1 file changed, 2 insertions(+)
arch/sparc/include/asm/compat.h +2 -0
@@ -5,6 +5,8 @@* Architecture specific compatibility types */ #include <linux/types.h> +#include <linux/thread_info.h> +#include <asm/thread_info.h> #define COMPAT_USER_HZ 100#define COMPAT_UTS_MACHINE "sparc\0\0"
Signature · 1 line
2.17.1
PATCH v3 3/7 riscv: Delete asm/compat.h
In reply to Deepa Dinamaniriscv does not enable CONFIG_COMPAT in default configurations:
defconfig, allmodconfig and allnoconfig.
Remove the asm/compat.h as it does not seem to add any value to the architecture without CONFIG_COMPAT.
Now that time compat syscalls are being reused in non CONFIG_COMPAT modes, asm-generic/compat.h provides definitions for riscv 32 bit mode.
Alternative would be to make compat_time.h to be conditional on CONFIG_COMPAT_32BIT_TIME. But, since riscv does not does not need asm/compat.h, delete it instead.
--- arch/riscv/include/asm/Kbuild | 1 + arch/riscv/include/asm/compat.h | 29 ----------------------------- 2 files changed, 1 insertion(+), 29 deletions(-) delete mode 100644 arch/riscv/include/asm/compat.h
arch/riscv/include/asm/Kbuild +1 -0
@@ -1,6 +1,7 @@ generic-y += bugs.h generic-y += cacheflush.h generic-y += checksum.h +generic-y += compat.h generic-y += cputime.h generic-y += device.h generic-y += div64.h
arch/riscv/include/asm/compat.h deleted +0 -29
@@ -1,29 +0,0 @@ -/* -* Copyright (C) 2012 ARM Ltd. -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see <http://www.gnu.org/licenses/>. -*/ -#ifndef __ASM_COMPAT_H -#define __ASM_COMPAT_H -#ifdef CONFIG_COMPAT - -#if defined(CONFIG_64BIT ) -#define COMPAT_UTS_MACHINE "riscv64\0\0" -#elif defined(CONFIG_32BIT ) -#define COMPAT_UTS_MACHINE "riscv32\0\0" -#else -#error "Unknown RISC-V base ISA" -#endif - -#endif /*CONFIG_COMPAT*/ -#endif /*__ASM_COMPAT_H*/
Signature · 1 line
2.17.1
Re: [PATCH v3 3/7] riscv: Delete asm/compat.h
Quoted 4 lines · Deepa Dinamani
riscv does not enable
CONFIG_COMPATin default configurations:
defconfig, allmodconfig and allnoconfig.
Remove theasm/compat.has it does not seem to add any value to the architecture withoutCONFIG_COMPAT.
Looks good,
Quoted 8 lines
Now that time compat syscalls are being reused in non
CONFIG_COMPATmodes,asm-generic/compat.hprovides definitions for riscv 32 bit mode.Alternative would be to make
compat_time.hto be conditional onCONFIG_COMPAT_32BIT_TIME. But, since riscv does not does not needasm/compat.h, delete it instead.
Except that all this isn't relavant for this patch (and a horribly bad idea, but ?'ll reply to it separately).
Re: [PATCH v3 3/7] riscv: Delete asm/compat.h
In reply to Deepa DinamaniQuoted 59 lines · deepa.kernel at gmail.com
riscv does not enable
CONFIG_COMPATin default configurations:
defconfig, allmodconfig and allnoconfig.
Remove theasm/compat.has it does not seem to add any value to the architecture withoutCONFIG_COMPAT.Now that time compat syscalls are being reused in non
CONFIG_COMPATmodes,asm-generic/compat.hprovides definitions for riscv 32 bit mode.Alternative would be to make
compat_time.hto be conditional onCONFIG_COMPAT_32BIT_TIME. But, since riscv does not does not needasm/compat.h, delete it instead.Signed-off-by Deepa Dinamani <[email protected]>Cc palmer at sifive.comCc linux-riscv at lists.infradead.org--- arch/riscv/include/asm/Kbuild | 1 + arch/riscv/include/asm/compat.h | 29 ----------------------------- 2 files changed, 1 insertion(+), 29 deletions(-) delete mode 100644 arch/riscv/include/asm/compat.harch/riscv/include/asm/Kbuild +1
-0@@ -1,6 +1,7 @@ generic-y += bugs.h generic-y += cacheflush.h generic-y += checksum.h +generic-y += compat.h generic-y += cputime.h generic-y += device.h generic-y += div64.harch/riscv/include/asm/compat.h deleted +0
-19@@ -1,29 +0,0 @@ -/* -* Copyright (C) 2012 ARM Ltd. -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see <http://www.gnu.org/licenses/>. -*/ -#ifndef __ASM_COMPAT_H -#define __ASM_COMPAT_H -#ifdef CONFIG_COMPAT -
arch/riscv/include/asm/compat.h line 29
Quoted 10 lines
arch/riscv/include/asm/compat.h deleted +0
-10-#if defined(CONFIG_64BIT ) -#define COMPAT_UTS_MACHINE "riscv64\0\0" -#elif defined(CONFIG_32BIT ) -#define COMPAT_UTS_MACHINE "riscv32\0\0" -#else -#error "Unknown RISC-V base ISA" -#endif - -#endif /*CONFIG_COMPAT*/ -#endif /*__ASM_COMPAT_H*/
I thought we were using this to make uname print "riscv64" (or "riscv32") instead of just "riscv"? Is there another mechanism to do that?
Re: [PATCH v3 3/7] riscv: Delete asm/compat.h
Quoted 2 lines · Palmer Dabbelt
I thought we were using this to make uname print "riscv64" (or "riscv32") instead of just "riscv"? Is there another mechanism to do that?
That is set in the UTS_MACHINE variable in arch/riscv/Makefile and doesn't involve compat.h at all except when actually implementing 32-bit compat on 64 bit, which RISC-V doesn't (at least yet).
Arnd Bergmann
maintainerRe: [PATCH v3 3/7] riscv: Delete asm/compat.h
Quoted 7 lines · Christoph Hellwig
Quoted 2 lines · Palmer Dabbelt
I thought we were using this to make uname print "riscv64" (or "riscv32") instead of just "riscv"? Is there another mechanism to do that?
That is set in the
UTS_MACHINEvariable inarch/riscv/Makefileand doesn't involvecompat.hat all except when actually implementing 32-bit compat on 64 bit, which RISC-V doesn't (at least yet).
Note that the logic was also wrong, if we had an implementation of CONFIG_COMPAT for arch/riscv, it would always result in the "riscv64" string for both PER_LINUX and PER_LINUX32, when it should be "riscv32" for the PER_LINUX32 personaliy.
Arnd
PATCH v3 4/7 timex: prepare compat helpers for y2038 changes
In reply to Deepa DinamaniMove struct compat_timex to compat_time.h.
This is in line with other compat time structures that are planned to be deprecated eventually.
Also enable compat_get/put_timex helpers unconditionally.
--- include/asm-generic/compat.h | 8 ++++- include/linux/compat.h | 33 ------------------- include/linux/compat_time.h | 34 +++++++++++++++++++ kernel/compat.c | 63 ------------------------------------ kernel/time/time.c | 63 ++++++++++++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 97 deletions(-)
include/asm-generic/compat.h +7 -1
@@ -1,3 +1,9 @@/* SPDX-License-Identifier: GPL-2.0 */ -/* This is an empty stub for 32-bit-only architectures */ +/* This is a stub for 32-bit-only architectures */ + +#include <linux/types.h> + +typedef s32 compat_int_t ; +typedef s32 compat_long_t ; +typedef u32 compat_uint_t ;
include/linux/compat.h +0 -33
@@ -19,8 +19,6 @@#include <linux/uaccess.h> #include <linux/unistd.h> -#include <asm/compat.h> -#ifdef CONFIG_COMPAT #include <asm/siginfo.h> #include <asm/signal.h>
@@ -136,37 +134,6 @@ struct compat_tms {compat_clock_t tms_cstime; }; -struct compat_timex { -compat_uint_t modes; -compat_long_t offset; -compat_long_t freq; -compat_long_t maxerror; -compat_long_t esterror; -compat_int_t status; -compat_long_t constant; -compat_long_t precision; -compat_long_t tolerance; -struct compat_timeval time; -compat_long_t tick; -compat_long_t ppsfreq; -compat_long_t jitter; -compat_int_t shift; -compat_long_t stabil; -compat_long_t jitcnt; -compat_long_t calcnt; -compat_long_t errcnt; -compat_long_t stbcnt; -compat_int_t tai; - - compat_int_t:32; compat_int_t:32; compat_int_t:32; compat_int_t:32; - compat_int_t:32; compat_int_t:32; compat_int_t:32; compat_int_t:32; - compat_int_t:32; compat_int_t:32; compat_int_t:32; -}; - -struct timex ; -int compat_get_timex (struct timex *,const struct compat_timex __user *); -int compat_put_timex (struct compat_timex __user *,const struct timex *); -#define _COMPAT_NSIG_WORDS (_COMPAT_NSIG / _COMPAT_NSIG_BPW) typedef struct {
include/linux/compat_time.h +34 -0
@@ -7,6 +7,9 @@typedef s32 compat_time_t ; +/* TODO: Move to linux/compat.h when this file is deleted. */ +#include <asm/compat.h> +struct compat_timespec {compat_time_t tv_sec;s32 tv_nsec;
@@ -22,11 +25,42 @@ struct compat_itimerspec {struct compat_timespec it_value; }; +struct compat_timex { +compat_uint_t modes; +compat_long_t offset; +compat_long_t freq; +compat_long_t maxerror; +compat_long_t esterror; +compat_int_t status; +compat_long_t constant; +compat_long_t precision; +compat_long_t tolerance; +struct compat_timeval time; +compat_long_t tick; +compat_long_t ppsfreq; +compat_long_t jitter; +compat_int_t shift; +compat_long_t stabil; +compat_long_t jitcnt; +compat_long_t calcnt; +compat_long_t errcnt; +compat_long_t stbcnt; +compat_int_t tai; + + compat_int_t:32; compat_int_t:32; compat_int_t:32; compat_int_t:32; + compat_int_t:32; compat_int_t:32; compat_int_t:32; compat_int_t:32; + compat_int_t:32; compat_int_t:32; compat_int_t:32; +}; + +extern int compat_get_timespec64 (struct timespec64 *,const void __user *);extern int compat_put_timespec64 (const struct timespec64 *,void __user *);extern int get_compat_itimerspec64 (struct itimerspec64 *its,const struct compat_itimerspec __user *uits);extern int put_compat_itimerspec64 (const struct itimerspec64 *its,struct compat_itimerspec __user *uits); +struct timex ; +int compat_get_timex (struct timex *,const struct compat_timex __user *); +int compat_put_timex (struct compat_timex __user *,const struct timex *);#endif /* _LINUX_COMPAT_TIME_H */
kernel/compat.c +0 -63
69 lines not on this page · raw message
kernel/time/time.c +63 -0
66 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v3 5/7 time: Add struct __kernel_timex
In reply to Deepa Dinamanistruct timex uses struct timeval internally. struct timeval is not y2038 safe.
Introduce a new UAPI type struct __kernel_timex that is y2038 safe.
struct __kernel_timex uses a timeval type that is similar to struct __kernel_timespec which preserves the same structure size across 32 bit and 64 bit ABIs. struct __kernel_timex also restructures other members of the structure to make the structure the same on 64 bit and 32 bit architectures.
Note that struct __kernel_timex is the same as struct timex on a 64 bit architecture.
The above solution is similar to other new y2038 syscalls that are being introduced: both 32 bit and 64 bit ABIs have a common entry, and the compat entry supports the old 32 bit syscall interface.
Alternatives considered were:
1. Add new time type to struct timex that makes use of padded bits. This time type could be based on the struct __kernel_timespec. modes will use a flag to notify which time structure should be used internally.
This needs some application level changes on both 64 bit and 32 bit architectures. Although 64 bit machines could continue to use the older timeval structure without any changes.
2. Add a new u8 type to struct timex that makes use of padded bits. This can be used to save higher order tv_sec bits. modes will use a flag to notify presence of such a type.
This will need some application level changes on 32 bit architectures.
3. Add a new compat_timex structure that differs in only the size of the time type; keep rest of struct timex the same.
This requires extra syscalls to manage all 3 cases on 64 bit architectures. This will not need any application level changes but will add more complexity from kernel side.
--- include/linux/timex.h | 7 +++++++ include/uapi/linux/timex.h | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+)
include/linux/timex.h +7 -0
@@ -53,6 +53,13 @@#ifndef _LINUX_TIMEX_H#define _LINUX_TIMEX_H +/* CONFIG_64BIT_TIME enables new 64 bit time_t syscalls in the compat path +* and 32-bit emulation. +*/ +#ifndef CONFIG_64BIT_TIME +#define __kernel_timex timex +#endif +#include <uapi/linux/timex.h> #define ADJ_ADJTIME 0x8000/* switch between adjtime/adjtimex modes */
include/uapi/linux/timex.h +41 -0
47 lines not on this page · raw message
Signature · 1 line
2.17.1
Arnd Bergmann
maintainerRe: [PATCH v3 5/7] time: Add struct __kernel_timex
Quoted 33 lines · Deepa Dinamani
struct timex uses struct timeval internally. struct timeval is not y2038 safe.
Introduce a new UAPI type struct__kernel_timexthat is y2038 safe.struct
__kernel_timexuses a timeval type that is similar to struct__kernel_timespecwhich preserves the same structure size across 32 bit and 64 bit ABIs. struct__kernel_timexalso restructures other members of the structure to make the structure the same on 64 bit and 32 bit architectures.
Note that struct__kernel_timexis the same as struct timex on a 64 bit architecture.The above solution is similar to other new y2038 syscalls that are being introduced: both 32 bit and 64 bit ABIs have a common entry, and the compat entry supports the old 32 bit syscall interface.
Alternatives considered were:
1. Add new time type to struct timex that makes use of padded bits. This time type could be based on the struct__kernel_timespec. modes will use a flag to notify which time structure should be used internally.
This needs some application level changes on both 64 bit and 32 bit architectures. Although 64 bit machines could continue to use the older timeval structure without any changes.2. Add a new u8 type to struct timex that makes use of padded bits. This can be used to save higher order
tv_secbits. modes will use a flag to notify presence of such a type.
This will need some application level changes on 32 bit architectures.
Quoted 5 lines
3. Add a new
compat_timexstructure that differs in only the size of the time type; keep rest of struct timex the same.
This requires extra syscalls to manage all 3 cases on 64 bit architectures. This will not need any application level changes but will add more complexity from kernel side.
Hi Deepa,
sorry for taking so long with my reply
I've done a similar patch for the itimerval and the rusuage structures that have the same basic problem of embedding a timeval, and after a lot of back-and-forth on my end, I came to the same conclusion with the structure layout: using a single binary layout that is shared between 32-bit and 64-bit ABIs is better than any of the other approaches that one of us tried.
However, I'm still undecided about how to exactly put that in the uapi headers.
Quoted 10 lines
#define
ADJ_ADJTIME0x8000/* switch between adjtime/adjtimex modes */include/uapi/linux/timex.h +1
-0@@ -92,6 +92,47 @@ struct timex { int :32; int :32; int :32; }; +struct __kernel_timex_timeval {
include/uapi/linux/timex.h struct timex { · line 105
Quoted 10 lines
include/uapi/linux/timex.h +10
-0+__kernel_time64_t tv_sec; +long long tv_usec; +}; + +#ifndef __kernel_timex +struct __kernel_timex { + unsigned int modes; /* mode selector */ + int :32; /* pad */ + long long offset; /* time offset (usec) */ + long long freq; /* frequency offset (scaled ppm) */
The main disadvantage here is that a a typical ntp daemon that includes this header will now call the new system call, but still see the old structure definition that no longer matches, unless one modifies it to use __kernel_timex.
I checked the most important libc implementations to see how they pass this structure to user space:
glibc, musl, uclibc:
sys/timex.h contains a private version of this structure, the kernel header is not included
bionic:
sys/timex.h includes linux/timex.h, which is shipped with the libc and generated from kernel headers
klibc:
there is no sys/timex.h, but also no adjtimex()/clock_adjtime() syscall
In short, this probably only really matters for bionic, and of course only if they choose to even support 64-bit time_t (which in turn requires other changes to the libc)
I can see two possible ways to define 'struct timex' in the kernel in a way that always matches the respective adjtimex.
a) rename the old timex to __kernel_old_timex, and add a snippet that will #define one of the two to be called 'struct timex' again:
#ifdef __USE_TIME_BITS64
#define __kernel_old_timex timex
#define __kernel_old_timex_timeval timeval
#else
#define __kernel_timex timex
#define __kernel_timex_timeval timeval
#endif
b) use only one definition in the header file, but use configuration dependent types, don't rename timex to __kernel_timex, and only use compat_timex for the old 32-bit version inside of the kernel:
/* for fields that are the same size as time_t */
#ifdef __KERNEL__
typedef __s64 timex_long_t;
#else
typedef time_t timex_long_t;
#endif
struct timex {
unsigned int modes; /* mode selector */
char __pad1[sizeof(timex_long_t) - sizeof(int);
__timex_long_t offset; /* time offset (usec) */
__timex_long_t freq; /* frequency offset (scaled ppm) */
__timex_long_t maxerror; /* maximum error (usec) */
__timex_long_t esterror; /* estimated error (usec) */
int status; /* clock command/status */
char __pad1[sizeof(timex_long_t) - sizeof(int);
__timex_long_t constant; /* pll time constant */
...
};
Both a) and b) should work fine with bionic and any application that includes linux/timex.h instead of sys/timex.h
Arnd
Re: [PATCH v3 5/7] time: Add struct __kernel_timex
In reply to Deepa DinamaniI don't think this patches makes sense without the next one, which actually uses the structure.
Quoted 3 lines
+/*
CONFIG_64BIT_TIMEenables new 64 bittime_tsyscalls in the compat path
+ * and 32-bit emulation.
+ */
Wrong comment style, also the 'compat path is the 32 (or 31 in case of s390) bit emulation, so the comment seems rather confusing.
include/linux/timex.h line 61
Quoted 3 lines
include/linux/timex.h +3
-0+#ifndef CONFIG_64BIT_TIME +#define __kernel_timex timex +#endif
using #defines for structs has all kinds of ill effects. Why can't we aways use __kernel_timex for the in-kernel usage?
include/uapi/linux/timex.h struct timex { · line 103
Quoted 4 lines
include/uapi/linux/timex.h +4
-0+#ifndef __kernel_timex +struct __kernel_timex { + unsigned int modes; /* mode selector */ + int :32; /* pad */
Why do we need padding for a purely in-kernel structure?
Also the anonymous member syntax is rather odd and I don't remeber us using it anywhere else. Why here?
Arnd Bergmann
maintainerRe: [PATCH v3 5/7] time: Add struct __kernel_timex
Quoted 6 lines · Christoph Hellwig
include/linux/timex.h line 61
Quoted 3 lines
include/linux/timex.h +3
-0+#ifndef CONFIG_64BIT_TIME +#define __kernel_timex timex +#endif using #defines for structs has all kinds of ill effects. Why can't we aways use
__kernel_timexfor the in-kernel usage?
It's the same thing that Deepa did in the already merged series for the timespec interfaces, this is done in order to stage the conversion of 50 system calls and 20 architectures without having to do everything at once.
The system call entry points are changed to take a __kernel_timespec or __kernel_timex argument as the first step, and the conditional macro override makes that a NOP. At the same time, the existing compat entry points are modified so they have the exact same behavior but use the compat_timespec, compat_timex etc argument types (which we can rename, too, as discussed).
When a 32-bit architecture gets converted, it sets CONFIG_64BIT_TIME, which flips the normal syscall entry to use 64-bit time_t, and enables the compat syscalls to be built. The same arch specific patch must then change the syscall table to redirect the old syscall numbers to the compat handlers (which implement the 32-bit time_t arguments), and add new numbers pointing to the default handlers that now take the 64-bit time_t, see https://pastebin.com/F3QZdyin
This was all explained in a lot of detail when the first set of patches got merged for the clock_ syscalls, but I suppose it can all be explained again.
Quoted 9 lines
include/uapi/linux/timex.h struct timex { · line 103
Quoted 4 lines
include/uapi/linux/timex.h +4
-0+#ifndef __kernel_timex +struct __kernel_timex { + unsigned int modes; /* mode selector */ + int :32; /* pad */Why do we need padding for a purely in-kernel structure?
Also the anonymous member syntax is rather odd and I don't remeber us using it anywhere else. Why here?
This is in the uapi header, and the structure just matches the existing timex definition, which has the same oddity.
Arnd
PATCH v3 6/7 timex: use __kernel_timex internally
In reply to Deepa Dinamanistruct timex is not y2038 safe.
Replace all uses of timex with y2038 safe __kernel_timex.
Note that struct __kernel_timex is an ABI interface definition. We could define a new structure based on __kernel_timex that is only available internally instead. Right now, there isn't a strong motivation for this as the structure is isolated to a few defined struct timex interfaces and such a structure would be exactly the same as struct timex.
The patch was generated by the following coccinelle script:
virtual patch
@depends on patch forall@
identifier ts;
expression e;
@@
(
- struct timex ts;
+ struct __kernel_timex ts;
|
- struct timex ts = {};
+ struct __kernel_timex ts = {};
|
- struct timex ts = e;
+ struct __kernel_timex ts = e;
|
- struct timex *ts;
+ struct __kernel_timex *ts;
|
(memset \| copy_from_user \| copy_to_user \)(...,
- sizeof(struct timex))
+ sizeof(struct __kernel_timex))
)
@depends on patch forall@
identifier ts;
identifier fn;
@@
fn(...,
- struct timex *ts,
+ struct __kernel_timex *ts,
...) {
...
}
@depends on patch forall@
identifier ts;
identifier fn;
@@
fn(...,
- struct timex *ts) {
+ struct __kernel_timex *ts) {
...
}
--- arch/alpha/kernel/osf_sys.c | 2 +- drivers/ptp/ptp_clock.c | 2 +- include/linux/compat_time.h | 6 +++--- include/linux/posix-clock.h | 2 +- include/linux/timex.h | 2 +- kernel/time/ntp.c | 12 +++++++----- kernel/time/ntp_internal.h | 2 +- kernel/time/posix-clock.c | 2 +- kernel/time/posix-timers.c | 4 ++-- kernel/time/posix-timers.h | 2 +- kernel/time/time.c | 14 +++++++------- kernel/time/timekeeping.c | 4 ++-- 12 files changed, 28 insertions(+), 26 deletions(-)
arch/alpha/kernel/osf_sys.c +1 -1
@@ -1260,7 +1260,7 @@ struct timex32 { SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p) { -struct timex txc; +struct __kernel_timex txc;int ret;/* copy relevant bits of struct timex. */
drivers/ptp/ptp_clock.c +1 -1
@@ -121,7 +121,7 @@ static int ptp_clock_gettime(struct posix_clock *pc, struct timespec64 *tp)return err; } -static int ptp_clock_adjtime (struct posix_clock *pc,struct timex *tx) +static int ptp_clock_adjtime (struct posix_clock *pc,struct __kernel_timex *tx) { struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);struct ptp_clock_info *ops;
include/linux/compat_time.h +3 -3
@@ -59,8 +59,8 @@ extern int get_compat_itimerspec64(struct itimerspec64 *its, const struct compat_itimerspec __user *uits);extern int put_compat_itimerspec64 (const struct itimerspec64 *its,struct compat_itimerspec __user *uits); -struct timex ; -int compat_get_timex (struct timex *,const struct compat_timex __user *); -int compat_put_timex (struct compat_timex __user *,const struct timex *); +struct __kernel_timex ; +int compat_get_timex (struct __kernel_timex *,const struct compat_timex __user *); +int compat_put_timex (struct compat_timex __user *,const struct __kernel_timex *);#endif /* _LINUX_COMPAT_TIME_H */
include/linux/posix-clock.h +1 -1
@@ -51,7 +51,7 @@ struct posix_clock;struct posix_clock_operations {struct module *owner; -int (*clock_adjtime)(struct posix_clock *pc,struct timex *tx); +int (*clock_adjtime)(struct posix_clock *pc,struct __kernel_timex *tx);int (*clock_gettime)(struct posix_clock *pc,struct timespec64 *ts);
include/linux/timex.h +1 -1
@@ -158,7 +158,7 @@ extern unsigned long tick_nsec; /* SHIFTED_HZ period (nsec) */#define NTP_INTERVAL_FREQ (HZ)#define NTP_INTERVAL_LENGTH (NSEC_PER_SEC/NTP_INTERVAL_FREQ) -extern int do_adjtimex (struct timex *); +extern int do_adjtimex (struct __kernel_timex *);extern void hardpps (const struct timespec64 *,const struct timespec64 *);int read_current_timer (unsigned long *timer_val);
kernel/time/ntp.c +7 -5
@@ -189,7 +189,7 @@ static inline int is_error_status(int status) && (status & (STA_PPSWANDER|STA_PPSERROR))); } -static inline void pps_fill_timex (struct timex *txc) +static inline void pps_fill_timex (struct __kernel_timex *txc) { txc->ppsfreq =shift_right ((pps_freq >>PPM_SCALE_INV_SHIFT ) *PPM_SCALE_INV ,NTP_SCALE_SHIFT );
@@ -221,7 +221,7 @@ static inline int is_error_status(int status)return status & (STA_UNSYNC |STA_CLOCKERR ); } -static inline void pps_fill_timex (struct timex *txc) +static inline void pps_fill_timex (struct __kernel_timex *txc) {/* PPS is not implemented, so these are zero */ txc->ppsfreq =0 ;
@@ -642,7 +642,8 @@ void ntp_notify_cmos_timer(void)/* * Propagate a new txc->status value into the NTP state: */ -static inline void process_adj_status (struct timex *txc,struct timespec64 *ts) +static inline void process_adj_status (struct __kernel_timex *txc, +struct timespec64 *ts) {if ((time_status &STA_PLL ) && !(txc->status &STA_PLL )) { time_state =TIME_OK ;
@@ -665,7 +666,7 @@ static inline void process_adj_status(struct timex *txc, struct timespec64 *ts) } -static inline void process_adjtimex_modes (struct timex *txc, +static inline void process_adjtimex_modes (struct __kernel_timex *txc,struct timespec64 *ts,s32 *time_tai) {
@@ -718,7 +719,8 @@ static inline void process_adjtimex_modes(struct timex *txc,* adjtimex mainly allows reading (and writing, if superuser) of * kernel time-keeping variables. used by xntpd. */ -int __do_adjtimex (struct timex *txc,struct timespec64 *ts,s32 *time_tai) +int __do_adjtimex (struct __kernel_timex *txc,struct timespec64 *ts, +s32 *time_tai) {int result;
kernel/time/ntp_internal.h +1 -1
@@ -8,6 +8,6 @@ extern void ntp_clear(void);extern u64 ntp_tick_length (void );extern ktime_t ntp_get_next_leap (void );extern int second_overflow (time64_t secs); -extern int __do_adjtimex (struct timex *,struct timespec64 *,s32 *); +extern int __do_adjtimex (struct __kernel_timex *,struct timespec64 *,s32 *);extern void __hardpps (const struct timespec64 *,const struct timespec64 *);#endif /* _LINUX_NTP_INTERNAL_H */
kernel/time/posix-clock.c +1 -1
@@ -241,7 +241,7 @@ static void put_clock_desc(struct posix_clock_desc *cd)fput (cd->fp); } -static int pc_clock_adjtime (clockid_t id,struct timex *tx) +static int pc_clock_adjtime (clockid_t id,struct __kernel_timex *tx) {struct posix_clock_desc cd;int err;
kernel/time/posix-timers.c +2 -2
@@ -209,7 +209,7 @@ static int posix_clock_realtime_set(const clockid_t which_clock, }static int posix_clock_realtime_adj (const clockid_t which_clock, -struct timex *t) +struct __kernel_timex *t) {return do_adjtimex (t); }
@@ -1087,7 +1087,7 @@ SYSCALL_DEFINE2(clock_adjtime, const clockid_t, which_clock, struct timex __user *, utx) {const struct k_clock *kc =clockid_to_kclock (which_clock); -struct timex ktx; +struct __kernel_timex ktx;int err;if (!kc)
kernel/time/posix-timers.h +1 -1
@@ -8,7 +8,7 @@ struct k_clock { const struct timespec64 *tp);int (*clock_get)(const clockid_t which_clock,struct timespec64 *tp); -int (*clock_adj)(const clockid_t which_clock,struct timex *tx); +int (*clock_adj)(const clockid_t which_clock,struct __kernel_timex *tx);int (*timer_create)(struct k_itimer *timer);int (*nsleep)(const clockid_t which_clock,int flags,const struct timespec64 *);
kernel/time/time.c +7 -7
@@ -265,24 +265,24 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct compat_timeval __user *, tv, SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p) { -struct timex txc;/* Local copy of parameter */ +struct __kernel_timex txc;/* Local copy of parameter */ int ret;/* Copy the user data space into the kernel copy * structure. But bear in mind that the structures * may change */ -if (copy_from_user (&txc, txc_p,sizeof (struct timex ))) +if (copy_from_user (&txc, txc_p,sizeof (struct __kernel_timex )))return -EFAULT ; ret =do_adjtimex (&txc); -return copy_to_user (txc_p, &txc,sizeof (struct timex )) ? -EFAULT : ret; +return copy_to_user (txc_p, &txc,sizeof (struct __kernel_timex )) ? -EFAULT : ret; }#ifdef CONFIG_COMPAT COMPAT_SYSCALL_DEFINE1(adjtimex, struct compat_timex __user *, utp) { -struct timex txc; +struct __kernel_timex txc;int err, ret; err =compat_get_timex (&txc, utp);
@@ -979,11 +979,11 @@ int put_compat_itimerspec64(const struct itimerspec64 *its, }EXPORT_SYMBOL_GPL (put_compat_itimerspec64); -int compat_get_timex (struct timex *txc,const struct compat_timex __user *utp) +int compat_get_timex (struct __kernel_timex *txc,const struct compat_timex __user *utp) {struct compat_timex tx32; -memset (txc,0 ,sizeof (struct timex )); +memset (txc,0 ,sizeof (struct __kernel_timex ));if (copy_from_user (&tx32, utp,sizeof (struct compat_timex )))return -EFAULT ;
@@ -1011,7 +1011,7 @@ int compat_get_timex(struct timex *txc, const struct compat_timex __user *utp)return 0 ; } -int compat_put_timex (struct compat_timex __user *utp,const struct timex *txc) +int compat_put_timex (struct compat_timex __user *utp,const struct __kernel_timex *txc) {struct compat_timex tx32;
kernel/time/timekeeping.c +2 -2
@@ -2220,7 +2220,7 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real,/** * timekeeping_validate_timex - Ensures the timex is ok for use in do_adjtimex */ -static int timekeeping_validate_timex (struct timex *txc) +static int timekeeping_validate_timex (struct __kernel_timex *txc) {if (txc->modes &ADJ_ADJTIME ) {/* singleshot must not be used with any other mode bits */
@@ -2286,7 +2286,7 @@ static int timekeeping_validate_timex(struct timex *txc)/** * do_adjtimex() - Accessor function to NTP __do_adjtimex function */ -int do_adjtimex (struct timex *txc) +int do_adjtimex (struct __kernel_timex *txc) {struct timekeeper *tk = &tk_core.timekeeper;unsigned long flags;
Signature · 1 line
2.17.1
Re: [PATCH v3 6/7] timex: use __kernel_timex internally
arch/alpha/kernel/osf_sys.c struct timex32 { · line 1266
Quoted 13 lines · Deepa Dinamani
arch/alpha/kernel/osf_sys.c +1
-1@@ -1260,7 +1260,7 @@ struct timex32 { SYSCALL_DEFINE1(old_adjtimex, struct timex32 __user *, txc_p) { -struct timex txc; +struct __kernel_timex txc;int ret;/* copy relevant bits of struct timex. */
r~
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
Re: [PATCH v3 6/7] timex: use __kernel_timex internally
In reply to Deepa DinamaniQuoted 4 lines · Deepa Dinamani
struct timex is not y2038 safe.
Replace all uses of timex with y2038 safe__kernel_timex.Note that struct
__kernel_timexis an ABI interface definition.
If it actually is an ABI interface it should probably have a different name.
PATCH v3 7/7 timex: change syscalls to use struct __kernel_timex
In reply to Deepa Dinamanistruct timex is not y2038 safe.
Switch all the syscall apis to use y2038 safe __kernel_timex.
Note that sys_adjtimex() does not have a y2038 safe solution. The api is meant to be deprecated on 32 bit machines after y2038.
--- include/linux/syscalls.h | 5 ++--- kernel/time/posix-timers.c | 10 +--------- kernel/time/time.c | 9 +++++++-- 3 files changed, 10 insertions(+), 14 deletions(-)
include/linux/syscalls.h +2 -3
@@ -54,7 +54,6 @@ struct __sysctl_args;struct sysinfo ;struct timespec ;struct timeval ; -struct timex ;struct timezone ;struct tms ;struct utimbuf ;
@@ -677,7 +676,7 @@ asmlinkage long sys_gettimeofday(struct timeval __user *tv, struct timezone __user *tz);asmlinkage long sys_settimeofday (struct timeval __user *tv,struct timezone __user *tz); -asmlinkage long sys_adjtimex (struct timex __user *txc_p); +asmlinkage long sys_adjtimex (struct __kernel_timex __user *txc_p);/* kernel/timer.c */ asmlinkage long sys_getpid (void );
@@ -846,7 +845,7 @@ asmlinkage long sys_open_by_handle_at(int mountdirfd,struct file_handle __user *handle, int flags);asmlinkage long sys_clock_adjtime (clockid_t which_clock, -struct timex __user *tx); +struct __kernel_timex __user *tx);asmlinkage long sys_syncfs (int fd);asmlinkage long sys_setns (int fd,int nstype); asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
kernel/time/posix-timers.c +1 -9
@@ -1084,7 +1084,7 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock, } SYSCALL_DEFINE2(clock_adjtime, const clockid_t, which_clock, - struct timex __user *, utx) + struct __kernel_timex __user *, utx) {const struct k_clock *kc =clockid_to_kclock (which_clock);struct __kernel_timex ktx;
@@ -1159,10 +1159,6 @@ COMPAT_SYSCALL_DEFINE2(clock_gettime, clockid_t, which_clock,return err; } -#endif - -#ifdef CONFIG_COMPAT -COMPAT_SYSCALL_DEFINE2 (clock_adjtime, clockid_t, which_clock, struct compat_timex __user *, utp) {
@@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,return err; } -#endif - -#ifdef CONFIG_COMPAT_32BIT_TIME -COMPAT_SYSCALL_DEFINE2 (clock_getres, clockid_t, which_clock, struct compat_timespec __user *, tp) {
kernel/time/time.c +7 -2
@@ -263,7 +263,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct compat_timeval __user *, tv, }#endif -SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p) + +#if !defined(CONFIG_64BIT_TIME ) || defined(CONFIG_64BIT ) + +SYSCALL_DEFINE1(adjtimex, struct __kernel_timex __user *, txc_p) {struct __kernel_timex txc;/* Local copy of parameter */ int ret;
@@ -278,7 +281,9 @@ SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)return copy_to_user (txc_p, &txc,sizeof (struct __kernel_timex )) ? -EFAULT : ret; } -#ifdef CONFIG_COMPAT +#endif + +#ifdef CONFIG_COMPAT_32BIT_TIME COMPAT_SYSCALL_DEFINE1(adjtimex, struct compat_timex __user *, utp) {
Signature · 1 line
2.17.1
Re: [PATCH v3 7/7] timex: change syscalls to use struct __kernel_timex
Quoted 2 lines · Deepa Dinamani
struct timex is not y2038 safe.
Switch all the syscall apis to use y2038 safe__kernel_timex.
So you switch existing syscalls to use a different structure. If this actually happens to be safe it needs a big explanation in the commit log.
kernel/time/posix-timers.c COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock, · line 1190
Quoted 10 lines
kernel/time/posix-timers.c +0
-3-#ifdef CONFIG_COMPAT -COMPAT_SYSCALL_DEFINE2 (clock_adjtime, clockid_t, which_clock, struct compat_timex __user *, utp) {@@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,return err; } -#endif
And this unconditionally defines clock_adjtime, but doesn't actually seem to add callers, which looks rather odd. Same for other bits in the patch.
Arnd Bergmann
maintainerRe: [PATCH v3 7/7] timex: change syscalls to use struct __kernel_timex
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Deepa Dinamani
kernel/time/posix-timers.c +0
-2-#ifdef CONFIG_COMPAT -COMPAT_SYSCALL_DEFINE2 (clock_adjtime, clockid_t, which_clock, struct compat_timex __user *, utp) {@@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,return err; }
Quoted 5 lines
Review COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock, · line 1190
Quoted 1 line
(quoted diff) +0
-1-#endifAnd this unconditionally defines
clock_adjtime, but doesn't actually seem to add callers, which looks rather odd. Same for other bits in the patch.
It really just moves compat_sys_clock_adjtime() into the same #ifdef CONFIG_COMPAT_32BIT_TIME that hides the surrounding functions.
Currently CONFIG_COMPAT_32BIT_TIME is used only as a subset of CONFIG_COMPAT, and the plan was to have 32-bit architectures enable it later so they could get access to all the functions implementing 32-bit time with a patch similar to what I'm testing with, see https://pastebin.com/F3QZdyin for the current draft that I use for testing.
We already spent several review rounds coming just discussing the naming of the macros etc before we decided on the CONFIG_64BIT_TIME and CONFIG_COMPAT_32BIT_TIME
symbol, but if you have a better suggestion (as part of moving away from the compat_ naming), we can change all that again.
Arnd