* [PATCH v4 09/16] y2038: sparc: Extend sysvipc data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
sparc, uses a nonstandard variation of the generic sysvipc
data structures, intended to have the padding moved around
so it can deal with big-endian 32-bit user space that has
64-bit time_t.
Unlike most architectures, sparc actually succeeded in
defining this right for big-endian CPUs, but as everyone else
got it wrong, we just use the same hack everywhere.
This takes just take the same approach here that we have for
the asm-generic headers and adds separate 32-bit fields for the
upper halves of the timestamps, to let libc deal with the mess
in user space.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/sparc/include/asm/compat.h | 32 ++++++++++++++++----------------
arch/sparc/include/uapi/asm/msgbuf.h | 22 +++++++++-------------
arch/sparc/include/uapi/asm/sembuf.h | 16 +++++++---------
arch/sparc/include/uapi/asm/shmbuf.h | 21 +++++++++------------
4 files changed, 41 insertions(+), 50 deletions(-)
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 1910c44521e3..4eb51d2dae98 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -192,10 +192,10 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- unsigned int __pad1;
- compat_time_t sem_otime;
- unsigned int __pad2;
- compat_time_t sem_ctime;
+ unsigned int sem_otime_high;
+ unsigned int sem_otime;
+ unsigned int sem_ctime_high;
+ unsigned int sem_ctime;
u32 sem_nsems;
u32 __unused1;
u32 __unused2;
@@ -203,12 +203,12 @@ struct compat_semid64_ds {
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
- unsigned int __pad1;
- compat_time_t msg_stime;
- unsigned int __pad2;
- compat_time_t msg_rtime;
- unsigned int __pad3;
- compat_time_t msg_ctime;
+ unsigned int msg_stime_high;
+ unsigned int msg_stime;
+ unsigned int msg_rtime_high;
+ unsigned int msg_rtime;
+ unsigned int msg_ctime_high;
+ unsigned int msg_ctime;
unsigned int msg_cbytes;
unsigned int msg_qnum;
unsigned int msg_qbytes;
@@ -220,12 +220,12 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
- unsigned int __pad1;
- compat_time_t shm_atime;
- unsigned int __pad2;
- compat_time_t shm_dtime;
- unsigned int __pad3;
- compat_time_t shm_ctime;
+ unsigned int shm_atime_high;
+ unsigned int shm_atime;
+ unsigned int shm_dtime_high;
+ unsigned int shm_dtime;
+ unsigned int shm_ctime_high;
+ unsigned int shm_ctime;
compat_size_t shm_segsz;
compat_pid_t shm_cpid;
compat_pid_t shm_lpid;
diff --git a/arch/sparc/include/uapi/asm/msgbuf.h b/arch/sparc/include/uapi/asm/msgbuf.h
index b601c4f4d956..ffc46c211d6d 100644
--- a/arch/sparc/include/uapi/asm/msgbuf.h
+++ b/arch/sparc/include/uapi/asm/msgbuf.h
@@ -8,25 +8,22 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
-
-#if defined(__sparc__) && defined(__arch64__)
-# define PADDING(x)
-#else
-# define PADDING(x) unsigned int x;
-#endif
-
-
struct msqid64_ds {
struct ipc64_perm msg_perm;
- PADDING(__pad1)
+#if defined(__sparc__) && defined(__arch64__)
__kernel_time_t msg_stime; /* last msgsnd time */
- PADDING(__pad2)
__kernel_time_t msg_rtime; /* last msgrcv time */
- PADDING(__pad3)
__kernel_time_t msg_ctime; /* last change time */
+#else
+ unsigned long msg_stime_high;
+ unsigned long msg_stime; /* last msgsnd time */
+ unsigned long msg_rtime_high;
+ unsigned long msg_rtime; /* last msgrcv time */
+ unsigned long msg_ctime_high;
+ unsigned long msg_ctime; /* last change time */
+#endif
unsigned long msg_cbytes; /* current number of bytes on queue */
unsigned long msg_qnum; /* number of messages in queue */
unsigned long msg_qbytes; /* max number of bytes on queue */
@@ -35,5 +32,4 @@ struct msqid64_ds {
unsigned long __unused1;
unsigned long __unused2;
};
-#undef PADDING
#endif /* _SPARC_MSGBUF_H */
diff --git a/arch/sparc/include/uapi/asm/sembuf.h b/arch/sparc/include/uapi/asm/sembuf.h
index f49b0ffa0ab8..f3d309c2e1cd 100644
--- a/arch/sparc/include/uapi/asm/sembuf.h
+++ b/arch/sparc/include/uapi/asm/sembuf.h
@@ -8,25 +8,23 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
-#if defined(__sparc__) && defined(__arch64__)
-# define PADDING(x)
-#else
-# define PADDING(x) unsigned int x;
-#endif
struct semid64_ds {
struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
- PADDING(__pad1)
+#if defined(__sparc__) && defined(__arch64__)
__kernel_time_t sem_otime; /* last semop time */
- PADDING(__pad2)
__kernel_time_t sem_ctime; /* last change time */
+#else
+ unsigned long sem_otime_high;
+ unsigned long sem_otime; /* last semop time */
+ unsigned long sem_ctime_high;
+ unsigned long sem_ctime; /* last change time */
+#endif
unsigned long sem_nsems; /* no. of semaphores in array */
unsigned long __unused1;
unsigned long __unused2;
};
-#undef PADDING
#endif /* _SPARC64_SEMBUF_H */
diff --git a/arch/sparc/include/uapi/asm/shmbuf.h b/arch/sparc/include/uapi/asm/shmbuf.h
index 286631db705c..06618b84822d 100644
--- a/arch/sparc/include/uapi/asm/shmbuf.h
+++ b/arch/sparc/include/uapi/asm/shmbuf.h
@@ -8,24 +8,23 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
-#if defined(__sparc__) && defined(__arch64__)
-# define PADDING(x)
-#else
-# define PADDING(x) unsigned int x;
-#endif
-
struct shmid64_ds {
struct ipc64_perm shm_perm; /* operation perms */
- PADDING(__pad1)
+#if defined(__sparc__) && defined(__arch64__)
__kernel_time_t shm_atime; /* last attach time */
- PADDING(__pad2)
__kernel_time_t shm_dtime; /* last detach time */
- PADDING(__pad3)
__kernel_time_t shm_ctime; /* last change time */
+#else
+ unsigned long shm_atime_high;
+ unsigned long shm_atime; /* last attach time */
+ unsigned long shm_dtime_high;
+ unsigned long shm_dtime; /* last detach time */
+ unsigned long shm_ctime_high;
+ unsigned long shm_ctime; /* last change time */
+#endif
size_t shm_segsz; /* size of segment (bytes) */
__kernel_pid_t shm_cpid; /* pid of creator */
__kernel_pid_t shm_lpid; /* pid of last operator */
@@ -46,6 +45,4 @@ struct shminfo64 {
unsigned long __unused4;
};
-#undef PADDING
-
#endif /* _SPARC_SHMBUF_H */
--
2.9.0
^ permalink raw reply related
* [PATCH v4 08/16] y2038: parisc: Extend sysvipc data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
parisc, uses a nonstandard variation of the generic sysvipc
data structures, intended to have the padding moved around
so it can deal with big-endian 32-bit user space that has
64-bit time_t.
Unlike most architectures, parisc actually succeeded in
defining this right for big-endian CPUs, but as everyone else
got it wrong, we just use the same hack everywhere.
This takes just take the same approach here that we have for
the asm-generic headers and adds separate 32-bit fields for the
upper halves of the timestamps, to let libc deal with the mess
in user space.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/parisc/include/asm/compat.h | 32 ++++++++++++++++----------------
arch/parisc/include/uapi/asm/msgbuf.h | 33 ++++++++++++++++-----------------
arch/parisc/include/uapi/asm/sembuf.h | 16 ++++++++--------
arch/parisc/include/uapi/asm/shmbuf.h | 19 ++++++++-----------
4 files changed, 48 insertions(+), 52 deletions(-)
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 0cdfec8857bd..ab8a54771507 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -138,10 +138,10 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- unsigned int __unused1;
- compat_time_t sem_otime;
- unsigned int __unused2;
- compat_time_t sem_ctime;
+ unsigned int sem_otime_high;
+ unsigned int sem_otime;
+ unsigned int sem_ctime_high;
+ unsigned int sem_ctime;
compat_ulong_t sem_nsems;
compat_ulong_t __unused3;
compat_ulong_t __unused4;
@@ -149,12 +149,12 @@ struct compat_semid64_ds {
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
- unsigned int __unused1;
- compat_time_t msg_stime;
- unsigned int __unused2;
- compat_time_t msg_rtime;
- unsigned int __unused3;
- compat_time_t msg_ctime;
+ unsigned int msg_stime_high;
+ unsigned int msg_stime;
+ unsigned int msg_rtime_high;
+ unsigned int msg_rtime;
+ unsigned int msg_ctime_high;
+ unsigned int msg_ctime;
compat_ulong_t msg_cbytes;
compat_ulong_t msg_qnum;
compat_ulong_t msg_qbytes;
@@ -166,12 +166,12 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
- unsigned int __unused1;
- compat_time_t shm_atime;
- unsigned int __unused2;
- compat_time_t shm_dtime;
- unsigned int __unused3;
- compat_time_t shm_ctime;
+ unsigned int shm_atime_high;
+ unsigned int shm_atime;
+ unsigned int shm_dtime_high;
+ unsigned int shm_dtime;
+ unsigned int shm_ctime_high;
+ unsigned int shm_ctime;
unsigned int __unused4;
compat_size_t shm_segsz;
compat_pid_t shm_cpid;
diff --git a/arch/parisc/include/uapi/asm/msgbuf.h b/arch/parisc/include/uapi/asm/msgbuf.h
index b48b810e626b..6a2e9ab2ef8d 100644
--- a/arch/parisc/include/uapi/asm/msgbuf.h
+++ b/arch/parisc/include/uapi/asm/msgbuf.h
@@ -10,31 +10,30 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
struct msqid64_ds {
struct ipc64_perm msg_perm;
-#if __BITS_PER_LONG != 64
- unsigned int __pad1;
-#endif
+#if __BITS_PER_LONG == 64
__kernel_time_t msg_stime; /* last msgsnd time */
-#if __BITS_PER_LONG != 64
- unsigned int __pad2;
-#endif
__kernel_time_t msg_rtime; /* last msgrcv time */
-#if __BITS_PER_LONG != 64
- unsigned int __pad3;
-#endif
__kernel_time_t msg_ctime; /* last change time */
- unsigned long msg_cbytes; /* current number of bytes on queue */
- unsigned long msg_qnum; /* number of messages in queue */
- unsigned long msg_qbytes; /* max number of bytes on queue */
- __kernel_pid_t msg_lspid; /* pid of last msgsnd */
- __kernel_pid_t msg_lrpid; /* last receive pid */
- unsigned long __unused1;
- unsigned long __unused2;
+#else
+ unsigned long msg_stime_high;
+ unsigned long msg_stime; /* last msgsnd time */
+ unsigned long msg_rtime_high;
+ unsigned long msg_rtime; /* last msgrcv time */
+ unsigned long msg_ctime_high;
+ unsigned long msg_ctime; /* last change time */
+#endif
+ unsigned long msg_cbytes; /* current number of bytes on queue */
+ unsigned long msg_qnum; /* number of messages in queue */
+ unsigned long msg_qbytes; /* max number of bytes on queue */
+ __kernel_pid_t msg_lspid; /* pid of last msgsnd */
+ __kernel_pid_t msg_lrpid; /* last receive pid */
+ unsigned long __unused1;
+ unsigned long __unused2;
};
#endif /* _PARISC_MSGBUF_H */
diff --git a/arch/parisc/include/uapi/asm/sembuf.h b/arch/parisc/include/uapi/asm/sembuf.h
index 746c5d86a9b1..3c31163b1241 100644
--- a/arch/parisc/include/uapi/asm/sembuf.h
+++ b/arch/parisc/include/uapi/asm/sembuf.h
@@ -10,21 +10,21 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
struct semid64_ds {
struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
-#if __BITS_PER_LONG != 64
- unsigned int __pad1;
-#endif
+#if __BITS_PER_LONG == 64
__kernel_time_t sem_otime; /* last semop time */
-#if __BITS_PER_LONG != 64
- unsigned int __pad2;
-#endif
__kernel_time_t sem_ctime; /* last change time */
- unsigned long sem_nsems; /* no. of semaphores in array */
+#else
+ unsigned long sem_otime_high;
+ unsigned long sem_otime; /* last semop time */
+ unsigned long sem_ctime_high;
+ unsigned long sem_ctime; /* last change time */
+#endif
+ unsigned long sem_nsems; /* no. of semaphores in array */
unsigned long __unused1;
unsigned long __unused2;
};
diff --git a/arch/parisc/include/uapi/asm/shmbuf.h b/arch/parisc/include/uapi/asm/shmbuf.h
index cd4dbce55d0b..c89b3dd8db21 100644
--- a/arch/parisc/include/uapi/asm/shmbuf.h
+++ b/arch/parisc/include/uapi/asm/shmbuf.h
@@ -10,25 +10,22 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
struct shmid64_ds {
struct ipc64_perm shm_perm; /* operation perms */
-#if __BITS_PER_LONG != 64
- unsigned int __pad1;
-#endif
+#if __BITS_PER_LONG == 64
__kernel_time_t shm_atime; /* last attach time */
-#if __BITS_PER_LONG != 64
- unsigned int __pad2;
-#endif
__kernel_time_t shm_dtime; /* last detach time */
-#if __BITS_PER_LONG != 64
- unsigned int __pad3;
-#endif
__kernel_time_t shm_ctime; /* last change time */
-#if __BITS_PER_LONG != 64
+#else
+ unsigned long shm_atime_high;
+ unsigned long shm_atime; /* last attach time */
+ unsigned long shm_dtime_high;
+ unsigned long shm_dtime; /* last detach time */
+ unsigned long shm_ctime_high;
+ unsigned long shm_ctime; /* last change time */
unsigned int __pad4;
#endif
__kernel_size_t shm_segsz; /* size of segment (bytes) */
--
2.9.0
^ permalink raw reply related
* [PATCH v4 07/16] y2038: mips: Extend sysvipc data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
MIPS is the weirdest case for sysvipc, because each of the
three data structures is done differently:
* msqid64_ds has padding in the right place so we could in theory
extend this one to just have 64-bit values instead of time_t.
As this does not work for most of the other combinations,
we just handle it in the common manner though.
* semid64_ds has no padding for 64-bit time_t, but has two reserved
'long' fields, which are sufficient to extend the sem_otime
and sem_ctime fields to 64 bit. In order to do this, the libc
implementation will have to copy the data into another structure
that has the fields in a different order. MIPS is the only
architecture with this problem, so this is best done in MIPS
specific libc code.
* shmid64_ds is slightly worse than that, because it has three
time_t fields but only two unused 32-bit words. As a workaround,
we extend each field only by 16 bits, ending up with 48-bit
timestamps that user space again has to work around by itself.
The compat versions of the data structures are changed in the
same way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/mips/include/asm/compat.h | 40 ++++++++++++++------------
arch/mips/include/uapi/asm/msgbuf.h | 57 ++++++++++++++++++++++++-------------
arch/mips/include/uapi/asm/sembuf.h | 15 ++++++++--
arch/mips/include/uapi/asm/shmbuf.h | 23 +++++++++++++--
4 files changed, 94 insertions(+), 41 deletions(-)
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 3e548ee99a2f..78675f19440f 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -37,9 +37,11 @@ typedef struct {
typedef s32 compat_timer_t;
typedef s32 compat_key_t;
+typedef s16 compat_short_t;
typedef s32 compat_int_t;
typedef s32 compat_long_t;
typedef s64 compat_s64;
+typedef u16 compat_ushort_t;
typedef u32 compat_uint_t;
typedef u32 compat_ulong_t;
typedef u64 compat_u64;
@@ -157,35 +159,35 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- compat_time_t sem_otime;
- compat_time_t sem_ctime;
+ compat_ulong_t sem_otime;
+ compat_ulong_t sem_ctime;
compat_ulong_t sem_nsems;
- compat_ulong_t __unused1;
- compat_ulong_t __unused2;
+ compat_ulong_t sem_otime_high;
+ compat_ulong_t sem_ctime_high;
};
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
#ifndef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused1;
+ compat_ulong_t msg_stime_high;
#endif
- compat_time_t msg_stime;
+ compat_ulong_t msg_stime;
#ifdef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused1;
+ compat_ulong_t msg_stime_high;
#endif
#ifndef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused2;
+ compat_ulong_t msg_rtime_high;
#endif
- compat_time_t msg_rtime;
+ compat_ulong_t msg_rtime;
#ifdef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused2;
+ compat_ulong_t msg_rtime_high;
#endif
#ifndef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused3;
+ compat_ulong_t msg_ctime_high;
#endif
- compat_time_t msg_ctime;
+ compat_ulong_t msg_ctime;
#ifdef CONFIG_CPU_LITTLE_ENDIAN
- compat_ulong_t __unused3;
+ compat_ulong_t msg_ctime_high;
#endif
compat_ulong_t msg_cbytes;
compat_ulong_t msg_qnum;
@@ -199,14 +201,16 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
compat_size_t shm_segsz;
- compat_time_t shm_atime;
- compat_time_t shm_dtime;
- compat_time_t shm_ctime;
+ compat_ulong_t shm_atime;
+ compat_ulong_t shm_dtime;
+ compat_ulong_t shm_ctime;
compat_pid_t shm_cpid;
compat_pid_t shm_lpid;
compat_ulong_t shm_nattch;
- compat_ulong_t __unused1;
- compat_ulong_t __unused2;
+ compat_ushort_t shm_atime_high;
+ compat_ushort_t shm_dtime_high;
+ compat_ushort_t shm_ctime_high;
+ compat_ushort_t __unused2;
};
/* MIPS has unusual order of fields in stack_t */
diff --git a/arch/mips/include/uapi/asm/msgbuf.h b/arch/mips/include/uapi/asm/msgbuf.h
index eb4d0f9d7364..46aa15b13e4e 100644
--- a/arch/mips/include/uapi/asm/msgbuf.h
+++ b/arch/mips/include/uapi/asm/msgbuf.h
@@ -9,33 +9,15 @@
* between kernel and user space.
*
* Pad space is left for:
- * - extension of time_t to 64-bit on 32-bitsystem to solve the y2038 problem
* - 2 miscellaneous unsigned long values
*/
+#if defined(__mips64)
struct msqid64_ds {
struct ipc64_perm msg_perm;
-#if !defined(__mips64) && defined(__MIPSEB__)
- unsigned long __unused1;
-#endif
__kernel_time_t msg_stime; /* last msgsnd time */
-#if !defined(__mips64) && defined(__MIPSEL__)
- unsigned long __unused1;
-#endif
-#if !defined(__mips64) && defined(__MIPSEB__)
- unsigned long __unused2;
-#endif
__kernel_time_t msg_rtime; /* last msgrcv time */
-#if !defined(__mips64) && defined(__MIPSEL__)
- unsigned long __unused2;
-#endif
-#if !defined(__mips64) && defined(__MIPSEB__)
- unsigned long __unused3;
-#endif
__kernel_time_t msg_ctime; /* last change time */
-#if !defined(__mips64) && defined(__MIPSEL__)
- unsigned long __unused3;
-#endif
unsigned long msg_cbytes; /* current number of bytes on queue */
unsigned long msg_qnum; /* number of messages in queue */
unsigned long msg_qbytes; /* max number of bytes on queue */
@@ -44,5 +26,42 @@ struct msqid64_ds {
unsigned long __unused4;
unsigned long __unused5;
};
+#elif defined (__MIPSEB__)
+struct msqid64_ds {
+ struct ipc64_perm msg_perm;
+ unsigned long msg_stime_high;
+ unsigned long msg_stime; /* last msgsnd time */
+ unsigned long msg_rtime_high;
+ unsigned long msg_rtime; /* last msgrcv time */
+ unsigned long msg_ctime_high;
+ unsigned long msg_ctime; /* last change time */
+ unsigned long msg_cbytes; /* current number of bytes on queue */
+ unsigned long msg_qnum; /* number of messages in queue */
+ unsigned long msg_qbytes; /* max number of bytes on queue */
+ __kernel_pid_t msg_lspid; /* pid of last msgsnd */
+ __kernel_pid_t msg_lrpid; /* last receive pid */
+ unsigned long __unused4;
+ unsigned long __unused5;
+};
+#elif defined (__MIPSEL__)
+struct msqid64_ds {
+ struct ipc64_perm msg_perm;
+ unsigned long msg_stime; /* last msgsnd time */
+ unsigned long msg_stime_high;
+ unsigned long msg_rtime; /* last msgrcv time */
+ unsigned long msg_rtime_high;
+ unsigned long msg_ctime; /* last change time */
+ unsigned long msg_ctime_high;
+ unsigned long msg_cbytes; /* current number of bytes on queue */
+ unsigned long msg_qnum; /* number of messages in queue */
+ unsigned long msg_qbytes; /* max number of bytes on queue */
+ __kernel_pid_t msg_lspid; /* pid of last msgsnd */
+ __kernel_pid_t msg_lrpid; /* last receive pid */
+ unsigned long __unused4;
+ unsigned long __unused5;
+};
+#else
+#warning no endianess set
+#endif
#endif /* _ASM_MSGBUF_H */
diff --git a/arch/mips/include/uapi/asm/sembuf.h b/arch/mips/include/uapi/asm/sembuf.h
index 2c0f507ab7d1..60c89e6cb25b 100644
--- a/arch/mips/include/uapi/asm/sembuf.h
+++ b/arch/mips/include/uapi/asm/sembuf.h
@@ -7,10 +7,11 @@
* Note extra padding because this structure is passed back and forth
* between kernel and user space.
*
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
+ * Pad space is left for 2 miscellaneous 64-bit values on mips64,
+ * but used for the upper 32 bit of the time values on mips32.
*/
+#ifdef __mips64
struct semid64_ds {
struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
__kernel_time_t sem_otime; /* last semop time */
@@ -19,5 +20,15 @@ struct semid64_ds {
unsigned long __unused1;
unsigned long __unused2;
};
+#else
+struct semid64_ds {
+ struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
+ unsigned long sem_otime; /* last semop time */
+ unsigned long sem_ctime; /* last change time */
+ unsigned long sem_nsems; /* no. of semaphores in array */
+ unsigned long sem_otime_high;
+ unsigned long sem_ctime_high;
+};
+#endif
#endif /* _ASM_SEMBUF_H */
diff --git a/arch/mips/include/uapi/asm/shmbuf.h b/arch/mips/include/uapi/asm/shmbuf.h
index 379e6bca518b..9b9bba3401f2 100644
--- a/arch/mips/include/uapi/asm/shmbuf.h
+++ b/arch/mips/include/uapi/asm/shmbuf.h
@@ -7,10 +7,13 @@
* Note extra padding because this structure is passed back and forth
* between kernel and user space.
*
- * Pad space is left for:
- * - 2 miscellaneous 32-bit rsp. 64-bit values
+ * As MIPS was lacking proper padding after shm_?time, we use 48 bits
+ * of the padding at the end to store a few additional bits of the time.
+ * libc implementations need to take care to convert this into a proper
+ * data structure when moving to 64-bit time_t.
*/
+#ifdef __mips64
struct shmid64_ds {
struct ipc64_perm shm_perm; /* operation perms */
size_t shm_segsz; /* size of segment (bytes) */
@@ -23,6 +26,22 @@ struct shmid64_ds {
unsigned long __unused1;
unsigned long __unused2;
};
+#else
+struct shmid64_ds {
+ struct ipc64_perm shm_perm; /* operation perms */
+ size_t shm_segsz; /* size of segment (bytes) */
+ unsigned long shm_atime; /* last attach time */
+ unsigned long shm_dtime; /* last detach time */
+ unsigned long shm_ctime; /* last change time */
+ __kernel_pid_t shm_cpid; /* pid of creator */
+ __kernel_pid_t shm_lpid; /* pid of last operator */
+ unsigned long shm_nattch; /* no. of current attaches */
+ unsigned short shm_atime_high;
+ unsigned short shm_dtime_high;
+ unsigned short shm_ctime_high;
+ unsigned short __unused1;
+};
+#endif
struct shminfo64 {
unsigned long shmmax;
--
2.9.0
^ permalink raw reply related
* [PATCH v4 06/16] y2038: arm64: Extend sysvipc compat data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
Both 32-bit amd 64-bit ARM use the asm-generic header files for their
sysvipc data structures, so no special care is needed to make those
work beyond y2038, with the one exception of compat mode: Since there
is no asm-generic definition of the compat mode IPC structures, ARM64
provides its own copy, and we make those match the changes in the native
asm-generic header files.
There is sufficient padding in these data structures to extend all
timestamps to 64 bit, but on big-endian ARM kernels, the padding
is in the wrong place, so the C library has to ensure it reassembles
a 64-bit time_t correctly.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm64/include/asm/compat.h | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 0030f79808b3..1a037b94eba1 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -181,10 +181,10 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- compat_time_t sem_otime;
- compat_ulong_t __unused1;
- compat_time_t sem_ctime;
- compat_ulong_t __unused2;
+ compat_ulong_t sem_otime;
+ compat_ulong_t sem_otime_high;
+ compat_ulong_t sem_ctime;
+ compat_ulong_t sem_ctime_high;
compat_ulong_t sem_nsems;
compat_ulong_t __unused3;
compat_ulong_t __unused4;
@@ -192,12 +192,12 @@ struct compat_semid64_ds {
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
- compat_time_t msg_stime;
- compat_ulong_t __unused1;
- compat_time_t msg_rtime;
- compat_ulong_t __unused2;
- compat_time_t msg_ctime;
- compat_ulong_t __unused3;
+ compat_ulong_t msg_stime;
+ compat_ulong_t msg_stime_high;
+ compat_ulong_t msg_rtime;
+ compat_ulong_t msg_rtime_high;
+ compat_ulong_t msg_ctime;
+ compat_ulong_t msg_ctime_high;
compat_ulong_t msg_cbytes;
compat_ulong_t msg_qnum;
compat_ulong_t msg_qbytes;
@@ -210,12 +210,12 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
compat_size_t shm_segsz;
- compat_time_t shm_atime;
- compat_ulong_t __unused1;
- compat_time_t shm_dtime;
- compat_ulong_t __unused2;
- compat_time_t shm_ctime;
- compat_ulong_t __unused3;
+ compat_ulong_t shm_atime;
+ compat_ulong_t shm_atime_high;
+ compat_ulong_t shm_dtime;
+ compat_ulong_t shm_dtime_high;
+ compat_ulong_t shm_ctime;
+ compat_ulong_t shm_ctime_high;
compat_pid_t shm_cpid;
compat_pid_t shm_lpid;
compat_ulong_t shm_nattch;
--
2.9.0
^ permalink raw reply related
* [PATCH v4 05/16] y2038: s390: Remove unneeded ipc uapi header files
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
The s390 msgbuf/sembuf/shmbuf header files are all identical to the
version from asm-generic.
This patch removes the files and replaces them with 'generic-y'
statements, to avoid having to modify each copy when we extend sysvipc
to deal with 64-bit time_t in 32-bit user space.
Note that unlike alpha and ia64, the ipcbuf.h header file is slightly
different here, so I'm leaving the private copy.
To deal with 32-bit compat tasks, we also have to adapt the definitions
of compat_{shm,sem,msg}id_ds to match the changes to the respective
asm-generic files.
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/s390/include/asm/compat.h | 32 ++++++++++++------------
arch/s390/include/uapi/asm/Kbuild | 3 +++
arch/s390/include/uapi/asm/msgbuf.h | 38 ----------------------------
arch/s390/include/uapi/asm/sembuf.h | 30 -----------------------
arch/s390/include/uapi/asm/shmbuf.h | 49 -------------------------------------
5 files changed, 19 insertions(+), 133 deletions(-)
delete mode 100644 arch/s390/include/uapi/asm/msgbuf.h
delete mode 100644 arch/s390/include/uapi/asm/sembuf.h
delete mode 100644 arch/s390/include/uapi/asm/shmbuf.h
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 501aaff85304..97db2fba546a 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -232,10 +232,10 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- compat_time_t sem_otime;
- compat_ulong_t __pad1;
- compat_time_t sem_ctime;
- compat_ulong_t __pad2;
+ compat_ulong_t sem_otime;
+ compat_ulong_t sem_otime_high;
+ compat_ulong_t sem_ctime;
+ compat_ulong_t sem_ctime_high;
compat_ulong_t sem_nsems;
compat_ulong_t __unused1;
compat_ulong_t __unused2;
@@ -243,12 +243,12 @@ struct compat_semid64_ds {
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
- compat_time_t msg_stime;
- compat_ulong_t __pad1;
- compat_time_t msg_rtime;
- compat_ulong_t __pad2;
- compat_time_t msg_ctime;
- compat_ulong_t __pad3;
+ compat_ulong_t msg_stime;
+ compat_ulong_t msg_stime_high;
+ compat_ulong_t msg_rtime;
+ compat_ulong_t msg_rtime_high;
+ compat_ulong_t msg_ctime;
+ compat_ulong_t msg_ctime_high;
compat_ulong_t msg_cbytes;
compat_ulong_t msg_qnum;
compat_ulong_t msg_qbytes;
@@ -261,12 +261,12 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
compat_size_t shm_segsz;
- compat_time_t shm_atime;
- compat_ulong_t __pad1;
- compat_time_t shm_dtime;
- compat_ulong_t __pad2;
- compat_time_t shm_ctime;
- compat_ulong_t __pad3;
+ compat_ulong_t shm_atime;
+ compat_ulong_t shm_atime_high;
+ compat_ulong_t shm_dtime;
+ compat_ulong_t shm_dtime_high;
+ compat_ulong_t shm_ctime;
+ compat_ulong_t shm_ctime_high;
compat_pid_t shm_cpid;
compat_pid_t shm_lpid;
compat_ulong_t shm_nattch;
diff --git a/arch/s390/include/uapi/asm/Kbuild b/arch/s390/include/uapi/asm/Kbuild
index faef3f7e8353..e364873e0d10 100644
--- a/arch/s390/include/uapi/asm/Kbuild
+++ b/arch/s390/include/uapi/asm/Kbuild
@@ -9,9 +9,12 @@ generic-y += errno.h
generic-y += fcntl.h
generic-y += ioctl.h
generic-y += mman.h
+generic-y += msgbuf.h
generic-y += param.h
generic-y += poll.h
generic-y += resource.h
+generic-y += sembuf.h
+generic-y += shmbuf.h
generic-y += sockios.h
generic-y += swab.h
generic-y += termbits.h
diff --git a/arch/s390/include/uapi/asm/msgbuf.h b/arch/s390/include/uapi/asm/msgbuf.h
deleted file mode 100644
index 604f847cd68c..000000000000
--- a/arch/s390/include/uapi/asm/msgbuf.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _S390_MSGBUF_H
-#define _S390_MSGBUF_H
-
-/*
- * The msqid64_ds structure for S/390 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
- * - 2 miscellaneous 32-bit values
- */
-
-struct msqid64_ds {
- struct ipc64_perm msg_perm;
- __kernel_time_t msg_stime; /* last msgsnd time */
-#ifndef __s390x__
- unsigned long __unused1;
-#endif /* ! __s390x__ */
- __kernel_time_t msg_rtime; /* last msgrcv time */
-#ifndef __s390x__
- unsigned long __unused2;
-#endif /* ! __s390x__ */
- __kernel_time_t msg_ctime; /* last change time */
-#ifndef __s390x__
- unsigned long __unused3;
-#endif /* ! __s390x__ */
- unsigned long msg_cbytes; /* current number of bytes on queue */
- unsigned long msg_qnum; /* number of messages in queue */
- unsigned long msg_qbytes; /* max number of bytes on queue */
- __kernel_pid_t msg_lspid; /* pid of last msgsnd */
- __kernel_pid_t msg_lrpid; /* last receive pid */
- unsigned long __unused4;
- unsigned long __unused5;
-};
-
-#endif /* _S390_MSGBUF_H */
diff --git a/arch/s390/include/uapi/asm/sembuf.h b/arch/s390/include/uapi/asm/sembuf.h
deleted file mode 100644
index 3e917697b668..000000000000
--- a/arch/s390/include/uapi/asm/sembuf.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _S390_SEMBUF_H
-#define _S390_SEMBUF_H
-
-/*
- * The semid64_ds structure for S/390 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem (for !__s390x__)
- * - 2 miscellaneous 32-bit values
- */
-
-struct semid64_ds {
- struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
- __kernel_time_t sem_otime; /* last semop time */
-#ifndef __s390x__
- unsigned long __unused1;
-#endif /* ! __s390x__ */
- __kernel_time_t sem_ctime; /* last change time */
-#ifndef __s390x__
- unsigned long __unused2;
-#endif /* ! __s390x__ */
- unsigned long sem_nsems; /* no. of semaphores in array */
- unsigned long __unused3;
- unsigned long __unused4;
-};
-
-#endif /* _S390_SEMBUF_H */
diff --git a/arch/s390/include/uapi/asm/shmbuf.h b/arch/s390/include/uapi/asm/shmbuf.h
deleted file mode 100644
index 9cdce8d7ce60..000000000000
--- a/arch/s390/include/uapi/asm/shmbuf.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _S390_SHMBUF_H
-#define _S390_SHMBUF_H
-
-/*
- * The shmid64_ds structure for S/390 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem (for !__s390x__)
- * - 2 miscellaneous 32-bit values
- */
-
-struct shmid64_ds {
- struct ipc64_perm shm_perm; /* operation perms */
- size_t shm_segsz; /* size of segment (bytes) */
- __kernel_time_t shm_atime; /* last attach time */
-#ifndef __s390x__
- unsigned long __unused1;
-#endif /* ! __s390x__ */
- __kernel_time_t shm_dtime; /* last detach time */
-#ifndef __s390x__
- unsigned long __unused2;
-#endif /* ! __s390x__ */
- __kernel_time_t shm_ctime; /* last change time */
-#ifndef __s390x__
- unsigned long __unused3;
-#endif /* ! __s390x__ */
- __kernel_pid_t shm_cpid; /* pid of creator */
- __kernel_pid_t shm_lpid; /* pid of last operator */
- unsigned long shm_nattch; /* no. of current attaches */
- unsigned long __unused4;
- unsigned long __unused5;
-};
-
-struct shminfo64 {
- unsigned long shmmax;
- unsigned long shmmin;
- unsigned long shmmni;
- unsigned long shmseg;
- unsigned long shmall;
- unsigned long __unused1;
- unsigned long __unused2;
- unsigned long __unused3;
- unsigned long __unused4;
-};
-
-#endif /* _S390_SHMBUF_H */
--
2.9.0
^ permalink raw reply related
* [PATCH v4 04/16] y2038: ia64: Remove unneeded ipc uapi header files
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
The ia64 ipcbuf/msgbuf/sembuf/shmbuf header files are all identical
to the version from asm-generic.
This patch removes the files and replaces them with 'generic-y'
statements as part of the y2038 changes. While ia64 no longer has
a compat mode and doesn't need the file any more, it seem nicer
to clean this up anyway.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/ia64/include/uapi/asm/Kbuild | 4 ++++
arch/ia64/include/uapi/asm/ipcbuf.h | 2 --
arch/ia64/include/uapi/asm/msgbuf.h | 28 --------------------------
arch/ia64/include/uapi/asm/sembuf.h | 23 ----------------------
arch/ia64/include/uapi/asm/shmbuf.h | 39 -------------------------------------
5 files changed, 4 insertions(+), 92 deletions(-)
delete mode 100644 arch/ia64/include/uapi/asm/ipcbuf.h
delete mode 100644 arch/ia64/include/uapi/asm/msgbuf.h
delete mode 100644 arch/ia64/include/uapi/asm/sembuf.h
delete mode 100644 arch/ia64/include/uapi/asm/shmbuf.h
diff --git a/arch/ia64/include/uapi/asm/Kbuild b/arch/ia64/include/uapi/asm/Kbuild
index c0527cfc48f0..3982e673e967 100644
--- a/arch/ia64/include/uapi/asm/Kbuild
+++ b/arch/ia64/include/uapi/asm/Kbuild
@@ -2,5 +2,9 @@
include include/uapi/asm-generic/Kbuild.asm
generic-y += bpf_perf_event.h
+generic-y += ipcbuf.h
generic-y += kvm_para.h
+generic-y += msgbuf.h
generic-y += poll.h
+generic-y += sembuf.h
+generic-y += shmbuf.h
diff --git a/arch/ia64/include/uapi/asm/ipcbuf.h b/arch/ia64/include/uapi/asm/ipcbuf.h
deleted file mode 100644
index 90d6445a14df..000000000000
--- a/arch/ia64/include/uapi/asm/ipcbuf.h
+++ /dev/null
@@ -1,2 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#include <asm-generic/ipcbuf.h>
diff --git a/arch/ia64/include/uapi/asm/msgbuf.h b/arch/ia64/include/uapi/asm/msgbuf.h
deleted file mode 100644
index aa25df92d9dc..000000000000
--- a/arch/ia64/include/uapi/asm/msgbuf.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ASM_IA64_MSGBUF_H
-#define _ASM_IA64_MSGBUF_H
-
-/*
- * The msqid64_ds structure for IA-64 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct msqid64_ds {
- struct ipc64_perm msg_perm;
- __kernel_time_t msg_stime; /* last msgsnd time */
- __kernel_time_t msg_rtime; /* last msgrcv time */
- __kernel_time_t msg_ctime; /* last change time */
- unsigned long msg_cbytes; /* current number of bytes on queue */
- unsigned long msg_qnum; /* number of messages in queue */
- unsigned long msg_qbytes; /* max number of bytes on queue */
- __kernel_pid_t msg_lspid; /* pid of last msgsnd */
- __kernel_pid_t msg_lrpid; /* last receive pid */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-#endif /* _ASM_IA64_MSGBUF_H */
diff --git a/arch/ia64/include/uapi/asm/sembuf.h b/arch/ia64/include/uapi/asm/sembuf.h
deleted file mode 100644
index 6ed058760afc..000000000000
--- a/arch/ia64/include/uapi/asm/sembuf.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ASM_IA64_SEMBUF_H
-#define _ASM_IA64_SEMBUF_H
-
-/*
- * The semid64_ds structure for IA-64 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct semid64_ds {
- struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
- __kernel_time_t sem_otime; /* last semop time */
- __kernel_time_t sem_ctime; /* last change time */
- unsigned long sem_nsems; /* no. of semaphores in array */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-#endif /* _ASM_IA64_SEMBUF_H */
diff --git a/arch/ia64/include/uapi/asm/shmbuf.h b/arch/ia64/include/uapi/asm/shmbuf.h
deleted file mode 100644
index 6ef57cb70dee..000000000000
--- a/arch/ia64/include/uapi/asm/shmbuf.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ASM_IA64_SHMBUF_H
-#define _ASM_IA64_SHMBUF_H
-
-/*
- * The shmid64_ds structure for IA-64 architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct shmid64_ds {
- struct ipc64_perm shm_perm; /* operation perms */
- size_t shm_segsz; /* size of segment (bytes) */
- __kernel_time_t shm_atime; /* last attach time */
- __kernel_time_t shm_dtime; /* last detach time */
- __kernel_time_t shm_ctime; /* last change time */
- __kernel_pid_t shm_cpid; /* pid of creator */
- __kernel_pid_t shm_lpid; /* pid of last operator */
- unsigned long shm_nattch; /* no. of current attaches */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-struct shminfo64 {
- unsigned long shmmax;
- unsigned long shmmin;
- unsigned long shmmni;
- unsigned long shmseg;
- unsigned long shmall;
- unsigned long __unused1;
- unsigned long __unused2;
- unsigned long __unused3;
- unsigned long __unused4;
-};
-
-#endif /* _ASM_IA64_SHMBUF_H */
--
2.9.0
^ permalink raw reply related
* [PATCH v4 03/16] y2038: alpha: Remove unneeded ipc uapi header files
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
The alpha ipcbuf/msgbuf/sembuf/shmbuf header files are all identical
to the version from asm-generic.
This patch removes the files and replaces them with 'generic-y'
statements as part of the y2038 series. Since there is no 32-bit
syscall support for alpha, we don't need the other changes, but
it's good to have clean this up anyway.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/alpha/include/uapi/asm/Kbuild | 4 ++++
arch/alpha/include/uapi/asm/ipcbuf.h | 2 --
arch/alpha/include/uapi/asm/msgbuf.h | 28 --------------------------
arch/alpha/include/uapi/asm/sembuf.h | 23 ---------------------
arch/alpha/include/uapi/asm/shmbuf.h | 39 ------------------------------------
5 files changed, 4 insertions(+), 92 deletions(-)
delete mode 100644 arch/alpha/include/uapi/asm/ipcbuf.h
delete mode 100644 arch/alpha/include/uapi/asm/msgbuf.h
delete mode 100644 arch/alpha/include/uapi/asm/sembuf.h
delete mode 100644 arch/alpha/include/uapi/asm/shmbuf.h
diff --git a/arch/alpha/include/uapi/asm/Kbuild b/arch/alpha/include/uapi/asm/Kbuild
index 9afaba5e5503..1a5b75310cf4 100644
--- a/arch/alpha/include/uapi/asm/Kbuild
+++ b/arch/alpha/include/uapi/asm/Kbuild
@@ -2,4 +2,8 @@
include include/uapi/asm-generic/Kbuild.asm
generic-y += bpf_perf_event.h
+generic-y += ipcbuf.h
+generic-y += msgbuf.h
generic-y += poll.h
+generic-y += sembuf.h
+generic-y += shmbuf.h
diff --git a/arch/alpha/include/uapi/asm/ipcbuf.h b/arch/alpha/include/uapi/asm/ipcbuf.h
deleted file mode 100644
index 90d6445a14df..000000000000
--- a/arch/alpha/include/uapi/asm/ipcbuf.h
+++ /dev/null
@@ -1,2 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#include <asm-generic/ipcbuf.h>
diff --git a/arch/alpha/include/uapi/asm/msgbuf.h b/arch/alpha/include/uapi/asm/msgbuf.h
deleted file mode 100644
index 8c5d4d8c1b16..000000000000
--- a/arch/alpha/include/uapi/asm/msgbuf.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ALPHA_MSGBUF_H
-#define _ALPHA_MSGBUF_H
-
-/*
- * The msqid64_ds structure for alpha architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct msqid64_ds {
- struct ipc64_perm msg_perm;
- __kernel_time_t msg_stime; /* last msgsnd time */
- __kernel_time_t msg_rtime; /* last msgrcv time */
- __kernel_time_t msg_ctime; /* last change time */
- unsigned long msg_cbytes; /* current number of bytes on queue */
- unsigned long msg_qnum; /* number of messages in queue */
- unsigned long msg_qbytes; /* max number of bytes on queue */
- __kernel_pid_t msg_lspid; /* pid of last msgsnd */
- __kernel_pid_t msg_lrpid; /* last receive pid */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-#endif /* _ALPHA_MSGBUF_H */
diff --git a/arch/alpha/include/uapi/asm/sembuf.h b/arch/alpha/include/uapi/asm/sembuf.h
deleted file mode 100644
index f28ffa668b2f..000000000000
--- a/arch/alpha/include/uapi/asm/sembuf.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ALPHA_SEMBUF_H
-#define _ALPHA_SEMBUF_H
-
-/*
- * The semid64_ds structure for alpha architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct semid64_ds {
- struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
- __kernel_time_t sem_otime; /* last semop time */
- __kernel_time_t sem_ctime; /* last change time */
- unsigned long sem_nsems; /* no. of semaphores in array */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-#endif /* _ALPHA_SEMBUF_H */
diff --git a/arch/alpha/include/uapi/asm/shmbuf.h b/arch/alpha/include/uapi/asm/shmbuf.h
deleted file mode 100644
index 7e041ca2eb40..000000000000
--- a/arch/alpha/include/uapi/asm/shmbuf.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _ALPHA_SHMBUF_H
-#define _ALPHA_SHMBUF_H
-
-/*
- * The shmid64_ds structure for alpha architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 2 miscellaneous 64-bit values
- */
-
-struct shmid64_ds {
- struct ipc64_perm shm_perm; /* operation perms */
- size_t shm_segsz; /* size of segment (bytes) */
- __kernel_time_t shm_atime; /* last attach time */
- __kernel_time_t shm_dtime; /* last detach time */
- __kernel_time_t shm_ctime; /* last change time */
- __kernel_pid_t shm_cpid; /* pid of creator */
- __kernel_pid_t shm_lpid; /* pid of last operator */
- unsigned long shm_nattch; /* no. of current attaches */
- unsigned long __unused1;
- unsigned long __unused2;
-};
-
-struct shminfo64 {
- unsigned long shmmax;
- unsigned long shmmin;
- unsigned long shmmni;
- unsigned long shmseg;
- unsigned long shmall;
- unsigned long __unused1;
- unsigned long __unused2;
- unsigned long __unused3;
- unsigned long __unused4;
-};
-
-#endif /* _ALPHA_SHMBUF_H */
--
2.9.0
^ permalink raw reply related
* [PATCH v4 02/16] y2038: x86: Extend sysvipc data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
This extends the x86 copy of the sysvipc data structures to deal with
32-bit user space that has 64-bit time_t and wants to see timestamps
beyond 2038.
Fortunately, x86 has padding for this purpose in all the data structures,
so we can just add extra fields. With msgid64_ds and shmid64_ds, the
data structure is identical to the asm-generic version, which we have
already extended.
For some reason however, the 64-bit version of semid64_ds ended up with
extra padding, so I'm implementing the same approach as the asm-generic
version here, by using separate fields for the upper and lower halves
of the two timestamps.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/include/asm/compat.h | 32 ++++++++++++++++----------------
arch/x86/include/uapi/asm/sembuf.h | 11 ++++++++++-
2 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 7cd314b71c51..fb97cf7c4137 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -134,10 +134,10 @@ struct compat_ipc64_perm {
struct compat_semid64_ds {
struct compat_ipc64_perm sem_perm;
- compat_time_t sem_otime;
- compat_ulong_t __unused1;
- compat_time_t sem_ctime;
- compat_ulong_t __unused2;
+ compat_ulong_t sem_otime;
+ compat_ulong_t sem_otime_high;
+ compat_ulong_t sem_ctime;
+ compat_ulong_t sem_ctime_high;
compat_ulong_t sem_nsems;
compat_ulong_t __unused3;
compat_ulong_t __unused4;
@@ -145,12 +145,12 @@ struct compat_semid64_ds {
struct compat_msqid64_ds {
struct compat_ipc64_perm msg_perm;
- compat_time_t msg_stime;
- compat_ulong_t __unused1;
- compat_time_t msg_rtime;
- compat_ulong_t __unused2;
- compat_time_t msg_ctime;
- compat_ulong_t __unused3;
+ compat_ulong_t msg_stime;
+ compat_ulong_t msg_stime_high;
+ compat_ulong_t msg_rtime;
+ compat_ulong_t msg_rtime_high;
+ compat_ulong_t msg_ctime;
+ compat_ulong_t msg_ctime_high;
compat_ulong_t msg_cbytes;
compat_ulong_t msg_qnum;
compat_ulong_t msg_qbytes;
@@ -163,12 +163,12 @@ struct compat_msqid64_ds {
struct compat_shmid64_ds {
struct compat_ipc64_perm shm_perm;
compat_size_t shm_segsz;
- compat_time_t shm_atime;
- compat_ulong_t __unused1;
- compat_time_t shm_dtime;
- compat_ulong_t __unused2;
- compat_time_t shm_ctime;
- compat_ulong_t __unused3;
+ compat_ulong_t shm_atime;
+ compat_ulong_t shm_atime_high;
+ compat_ulong_t shm_dtime;
+ compat_ulong_t shm_dtime_high;
+ compat_ulong_t shm_ctime;
+ compat_ulong_t shm_ctime_high;
compat_pid_t shm_cpid;
compat_pid_t shm_lpid;
compat_ulong_t shm_nattch;
diff --git a/arch/x86/include/uapi/asm/sembuf.h b/arch/x86/include/uapi/asm/sembuf.h
index cabd7476bd6c..89de6cd9f0a7 100644
--- a/arch/x86/include/uapi/asm/sembuf.h
+++ b/arch/x86/include/uapi/asm/sembuf.h
@@ -8,15 +8,24 @@
* between kernel and user space.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
+ *
+ * x86_64 and x32 incorrectly added padding here, so the structures
+ * are still incompatible with the padding on x86.
*/
struct semid64_ds {
struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
+#ifdef __i386__
+ unsigned long sem_otime; /* last semop time */
+ unsigned long sem_otime_high;
+ unsigned long sem_ctime; /* last change time */
+ unsigned long sem_ctime_high;
+#else
__kernel_time_t sem_otime; /* last semop time */
__kernel_ulong_t __unused1;
__kernel_time_t sem_ctime; /* last change time */
__kernel_ulong_t __unused2;
+#endif
__kernel_ulong_t sem_nsems; /* no. of semaphores in array */
__kernel_ulong_t __unused3;
__kernel_ulong_t __unused4;
--
2.9.0
^ permalink raw reply related
* [PATCH v4 01/16] y2038: asm-generic: Extend sysvipc data structures
From: Arnd Bergmann @ 2018-04-25 15:05 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: linux-api, linux-arch, libc-alpha, tglx, deepa.kernel, viro,
ebiederm, albert.aribaud, linux-s390, schwidefsky, x86,
catalin.marinas, will.deacon, linux-mips, jhogan, ralf,
linuxppc-dev, sparclinux, zackw, noloader, Arnd Bergmann
In-Reply-To: <20180425132242.1500539-1-arnd@arndb.de>
Most architectures now use the asm-generic copy of the sysvipc data
structures (msqid64_ds, semid64_ds, shmid64_ds), which use 32-bit
__kernel_time_t on 32-bit architectures but have padding behind them to
allow extending the type to 64-bit.
Unfortunately, that fails on all big-endian architectures, which have the
padding on the wrong side. As so many of them get it wrong, we decided to
not bother even trying to fix it up when we introduced the asm-generic
copy. Instead we always use the padding word now to provide the upper
32 bits of the seconds value, regardless of the endianess.
A libc implementation on a typical big-endian system can deal with
this by providing its own copy of the structure definition to user
space, and swapping the two 32-bit words before returning from the
semctl/shmctl/msgctl system calls.
Note that msqid64_ds and shmid64_ds were broken on x32 since commit
f4b4aae18288 ("x86/headers/uapi: Fix __BITS_PER_LONG value for x32
builds"). I have sent a separate fix for that, but as we no longer
have to worry about x32 here, I no longer worry about x32 here and
use 'unsigned long' instead of __kernel_ulong_t.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/uapi/asm-generic/msgbuf.h | 27 +++++++++++++-------------
include/uapi/asm-generic/sembuf.h | 26 +++++++++++++++----------
include/uapi/asm-generic/shmbuf.h | 41 +++++++++++++++++++--------------------
3 files changed, 49 insertions(+), 45 deletions(-)
diff --git a/include/uapi/asm-generic/msgbuf.h b/include/uapi/asm-generic/msgbuf.h
index fb306ebdb36f..9fe4881557cb 100644
--- a/include/uapi/asm-generic/msgbuf.h
+++ b/include/uapi/asm-generic/msgbuf.h
@@ -18,31 +18,30 @@
* On big-endian systems, the padding is in the wrong place.
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
struct msqid64_ds {
struct ipc64_perm msg_perm;
+#if __BITS_PER_LONG == 64
__kernel_time_t msg_stime; /* last msgsnd time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused1;
-#endif
__kernel_time_t msg_rtime; /* last msgrcv time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused2;
-#endif
__kernel_time_t msg_ctime; /* last change time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused3;
+#else
+ unsigned long msg_stime; /* last msgsnd time */
+ unsigned long msg_stime_high;
+ unsigned long msg_rtime; /* last msgrcv time */
+ unsigned long msg_rtime_high;
+ unsigned long msg_ctime; /* last change time */
+ unsigned long msg_ctime_high;
#endif
- __kernel_ulong_t msg_cbytes; /* current number of bytes on queue */
- __kernel_ulong_t msg_qnum; /* number of messages in queue */
- __kernel_ulong_t msg_qbytes; /* max number of bytes on queue */
+ unsigned long msg_cbytes; /* current number of bytes on queue */
+ unsigned long msg_qnum; /* number of messages in queue */
+ unsigned long msg_qbytes; /* max number of bytes on queue */
__kernel_pid_t msg_lspid; /* pid of last msgsnd */
__kernel_pid_t msg_lrpid; /* last receive pid */
- __kernel_ulong_t __unused4;
- __kernel_ulong_t __unused5;
+ unsigned long __unused4;
+ unsigned long __unused5;
};
#endif /* __ASM_GENERIC_MSGBUF_H */
diff --git a/include/uapi/asm-generic/sembuf.h b/include/uapi/asm-generic/sembuf.h
index cbf9cfe977d6..0bae010f1b64 100644
--- a/include/uapi/asm-generic/sembuf.h
+++ b/include/uapi/asm-generic/sembuf.h
@@ -13,23 +13,29 @@
* everyone just ended up making identical copies without specific
* optimizations, so we may just as well all use the same one.
*
- * 64 bit architectures typically define a 64 bit __kernel_time_t,
+ * 64 bit architectures use a 64-bit __kernel_time_t here, while
+ * 32 bit architectures have a pair of unsigned long values.
* so they do not need the first two padding words.
- * On big-endian systems, the padding is in the wrong place.
*
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
- * - 2 miscellaneous 32-bit values
+ * On big-endian systems, the padding is in the wrong place for
+ * historic reasons, so user space has to reconstruct a time_t
+ * value using
+ *
+ * user_semid_ds.sem_otime = kernel_semid64_ds.sem_otime +
+ * ((long long)kernel_semid64_ds.sem_otime_high << 32)
+ *
+ * Pad space is left for 2 miscellaneous 32-bit values
*/
struct semid64_ds {
struct ipc64_perm sem_perm; /* permissions .. see ipc.h */
+#if __BITS_PER_LONG == 64
__kernel_time_t sem_otime; /* last semop time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused1;
-#endif
__kernel_time_t sem_ctime; /* last change time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused2;
+#else
+ unsigned long sem_otime; /* last semop time */
+ unsigned long sem_otime_high;
+ unsigned long sem_ctime; /* last change time */
+ unsigned long sem_ctime_high;
#endif
unsigned long sem_nsems; /* no. of semaphores in array */
unsigned long __unused3;
diff --git a/include/uapi/asm-generic/shmbuf.h b/include/uapi/asm-generic/shmbuf.h
index 2b6c3bb97f97..e504422fc501 100644
--- a/include/uapi/asm-generic/shmbuf.h
+++ b/include/uapi/asm-generic/shmbuf.h
@@ -19,42 +19,41 @@
*
*
* Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
* - 2 miscellaneous 32-bit values
*/
struct shmid64_ds {
struct ipc64_perm shm_perm; /* operation perms */
size_t shm_segsz; /* size of segment (bytes) */
+#if __BITS_PER_LONG == 64
__kernel_time_t shm_atime; /* last attach time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused1;
-#endif
__kernel_time_t shm_dtime; /* last detach time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused2;
-#endif
__kernel_time_t shm_ctime; /* last change time */
-#if __BITS_PER_LONG != 64
- unsigned long __unused3;
+#else
+ unsigned long shm_atime; /* last attach time */
+ unsigned long shm_atime_high;
+ unsigned long shm_dtime; /* last detach time */
+ unsigned long shm_dtime_high;
+ unsigned long shm_ctime; /* last change time */
+ unsigned long shm_ctime_high;
#endif
__kernel_pid_t shm_cpid; /* pid of creator */
__kernel_pid_t shm_lpid; /* pid of last operator */
- __kernel_ulong_t shm_nattch; /* no. of current attaches */
- __kernel_ulong_t __unused4;
- __kernel_ulong_t __unused5;
+ unsigned long shm_nattch; /* no. of current attaches */
+ unsigned long __unused4;
+ unsigned long __unused5;
};
struct shminfo64 {
- __kernel_ulong_t shmmax;
- __kernel_ulong_t shmmin;
- __kernel_ulong_t shmmni;
- __kernel_ulong_t shmseg;
- __kernel_ulong_t shmall;
- __kernel_ulong_t __unused1;
- __kernel_ulong_t __unused2;
- __kernel_ulong_t __unused3;
- __kernel_ulong_t __unused4;
+ unsigned long shmmax;
+ unsigned long shmmin;
+ unsigned long shmmni;
+ unsigned long shmseg;
+ unsigned long shmall;
+ unsigned long __unused1;
+ unsigned long __unused2;
+ unsigned long __unused3;
+ unsigned long __unused4;
};
#endif /* __ASM_GENERIC_SHMBUF_H */
--
2.9.0
^ permalink raw reply related
* y2038 kernel status update
From: Arnd Bergmann @ 2018-04-25 14:00 UTC (permalink / raw)
To: y2038 Mailman List
Cc: Linux Kernel Mailing List, Thomas Gleixner, Deepa Dinamani,
Baolin Wang, Albert ARIBAUD, Firoz Khan, RISC-V SW Dev, Linux API,
Private Kernel Alias
Hi everyone,
A lot of things have happened in the move to make the kernel y2038
ready in recent times. I've sent out new patches recently but will be
on vacation for the next few weeks, so here's a quick update on where
we are today with recent changes and work in progress:
- A large number of small driver fixes got merged into linux-4.16.
Many of these were old patches I had done a while ago but that had
not made it in yet. There are a few dozen more of these coming, but
those have some dependency.
- Deepa Dinamani has done a lot of the groundwork for the system
call conversion. Her latest patch series that got merged now finally
adds new system call entry points for timekeeping related syscalls. They
are not wired up yet, but now we have a template for the remaining ones,
and we have sorted out the header file dependencies.
- I just sent a pull request for a second chunk of system calls:
The sysvipc semtimedop, semctl/shmctl/msgctl and
mq_timedsend/mq_timedreceive calls. I have more patches for most of
the remaining calls, and they should be simpler (this one was rather
tricky). A full list can be found in a spreadsheet I made[2].
- Baolin Wang has worked on RTC related issues and most recently
posted a series to convert the user space ABI for ALSA. This is
currently under discussion.
- The most complicated remaining part is the conversion of
the VFS layer. Deepa is getting a new series for this ready after
having done some groundwork over the past years. The current idea is
to have a flag-day patch using coccinelle to change the inode, iattr
and kstat structures over at the same time. Many file systems will
require further changes after that, but those can be done individually
through their respective maintainers.
- Firoz Khan has started looking at the system call entry
points in the architectures. We want to add the new system calls to
all 32-bit architectures at once, in order to have a baseline release
that C libraries can rely on for implementing 64-bit time_t on all
architectures. Even after the architecture removal series, we still
have over a dozen different syscall tables and each one is laid out
slightly differently, which makes it hard to change. To improve this,
Firoz is preparing a series that changes
all architectures to use a common table format as used by x86, arm
and s390 today.
- I have collected all those patches in a git tree [1] for testing.
This currently has updated system call entry points for 32-bit ARM
and x86, but no others so far. The system call numbers *will* change
before it gets merged, so be careful when building user space based
on this, but it should be a starting point for testing. I definitely
expect this to contain bugs still, especially to the end of the series,
but the start of it should be fairly solid. Some patches are marked
as work-in-progress or don't contain a proper changelog text. Those
will still get reworked before a first version is posted to the
mainling lists.
- There are two notable areas not included in that tree today:
v4l and socket timestamps. I've had patches for both in the past but
they need to be reworked.
Arnd
[1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-next
[2] https://docs.google.com/spreadsheets/d/1HCYwHXxs48TsTb6IGUduNjQnmfRvMPzCN6T_0YiQwis/
^ permalink raw reply
* [GIT PULL, PATCH v4 00/16] y2038: IPC system call conversion
From: Arnd Bergmann @ 2018-04-25 13:22 UTC (permalink / raw)
To: y2038, linux-kernel
Cc: Arnd Bergmann, linux-api, linux-arch, libc-alpha, tglx,
deepa.kernel, viro, ebiederm, albert.aribaud, linux-s390,
schwidefsky, x86, catalin.marinas, will.deacon, linux-mips,
jhogan, ralf, linuxppc-dev, sparclinux, Zack Weinberg,
Jeffrey Walton
Hi Thomas,
This is a small update to last week's patch series, I hope I
have worked out all the remaining issues now. If nothing else
comes up, please pull into tip for 4.18. The commits are
based on top of what you already pulled into timers/core, so
you can either add these to the same branch or to a different
one.
I'll be away for three weeks starting on Sunday, so if new
issues get found, I won't be able to address them until I get
back home. I'll post another set of simpler syscall patches for
review after this, and there is at last a fourth set of those
that are not ready yet, but is available for preview at [3],
which also contains unrelated y2038 patches for random
subsystems.
Arnd
----------------------------------------------------------------
The following changes since commit 01909974b41036a6a8d3907c66cc7b41c9a73da9:
time: Change nanosleep to safe __kernel_* types (2018-04-19 13:32:03 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-ipc
for you to fetch changes up to 5dc0b1529d21b54aad4098874e334a52027fd16d:
y2038: ipc: Redirect ipc(SEMTIMEDOP, ...) to compat_ksys_semtimedop (2018-04-20 16:20:30 +0200)
----------------------------------------------------------------
y2038: IPC system call conversion
This is a follow-up to Deepa's work on the timekeeping system calls,
providing a y2038-safe syscall API for SYSVIPC. It uses a combination
of two strategies:
For sys_msgctl, sys_semctl and sys_shmctl, I do not introduce a completely
new set of replacement system calls, but instead extend the existing
ones to return data in the reserved fields of the normal data structure.
This should be completely transparent to any existing user space, and
only after the 32-bit time_t wraps, it will make a difference in the
returned data.
libc implementations will consequently have to provide their own data
structures when they move to 64-bit time_t, and convert the structures
in user space from the ones returned by the kernel.
In contrast, mq_timedsend, mq_timedreceive and and semtimedop all do
need to change because having a libc redefine the timespec type
breaks the ABI, so with this series there will be two separate entry
points for 32-bit architectures.
There are three cases here:
- little-endian architectures (except powerpc and mips) can use
the normal layout and just cast the data structure to the user space
type that contains 64-bit numbers.
- parisc and sparc can do the same thing with big-endian user space
- little-endian powerpc and most big-endian architectures have
to flip the upper and lower 32-bit halves of the time_t value in memory,
but can otherwise keep using the normal layout
- mips and big-endian xtensa need to be more careful because
they are not consistent in their definitions, and they have to provide
custom libc implementations for the system calls to use 64-bit time_t.
----------------------------------------------------------------
Changes to v3:
- reworked x86 portion after discovering an old bug, submitted
a fix for that separately.
- use consistent types in asm-generic based on feedback from
Jeffrey Walton
Changes to v2:
- added patches for mq_timedsend, mq_timedreceive and and semtimedop
system calls
- add asm-generic/compat.h changes to prepare for actually using those
on 32-bit.
- fix 'make headers_install' as reported by Heiko Carstens
- fix MIPS build as reported by build bot
- Cc everyone on all patches as originally intended, not just on the
cover letter.
Changes to v1 [1]:
- Rebased to the latest kernel (4.17-rc)
- Dropped changes for removed architectures
- Simplified the IPC code changes, based on prior work from
both Deepa and Eric
- Fixed a few bugs that I found during rebasing, in parcular the
sparc version was incorrect.
[1] https://lkml.org/lkml/2015/5/20/605
[2] https://lwn.net/Articles/751676/
[3] git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-next
----------------------------------------------------------------
Arnd Bergmann (16):
y2038: asm-generic: Extend sysvipc data structures
y2038: x86: Extend sysvipc data structures
y2038: alpha: Remove unneeded ipc uapi header files
y2038: ia64: Remove unneeded ipc uapi header files
y2038: s390: Remove unneeded ipc uapi header files
y2038: arm64: Extend sysvipc compat data structures
y2038: mips: Extend sysvipc data structures
y2038: parisc: Extend sysvipc data structures
y2038: sparc: Extend sysvipc data structures
y2038: powerpc: Extend sysvipc data structures
y2038: xtensa: Extend sysvipc data structures
y2038: ipc: Use ktime_get_real_seconds consistently
y2038: ipc: Report long times to user space
y2038: ipc: Use __kernel_timespec
y2038: ipc: Enable COMPAT_32BIT_TIME
y2038: ipc: Redirect ipc(SEMTIMEDOP, ...) to compat_ksys_semtimedop
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: libc-alpha@sourceware.org
Cc: tglx@linutronix.de
Cc: deepa.kernel@gmail.com
Cc: viro@zeniv.linux.org.uk
Cc: ebiederm@xmission.com
Cc: albert.aribaud@3adev.fr
Cc: linux-s390@vger.kernel.org
Cc: schwidefsky@de.ibm.com
Cc: x86@kernel.org
Cc: catalin.marinas@arm.com
Cc: will.deacon@arm.com
Cc: linux-mips@linux-mips.org
Cc: jhogan@kernel.org
Cc: ralf@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: Zack Weinberg <zackw@panix.com>
Cc: Jeffrey Walton <noloader@gmail.com>
arch/alpha/include/uapi/asm/Kbuild | 4 ++
arch/alpha/include/uapi/asm/ipcbuf.h | 2 -
arch/alpha/include/uapi/asm/msgbuf.h | 28 -----------
arch/alpha/include/uapi/asm/sembuf.h | 23 ---------
arch/alpha/include/uapi/asm/shmbuf.h | 39 ---------------
arch/arm64/include/asm/compat.h | 32 ++++++-------
arch/ia64/include/uapi/asm/Kbuild | 4 ++
arch/ia64/include/uapi/asm/ipcbuf.h | 2 -
arch/ia64/include/uapi/asm/msgbuf.h | 28 -----------
arch/ia64/include/uapi/asm/sembuf.h | 23 ---------
arch/ia64/include/uapi/asm/shmbuf.h | 39 ---------------
arch/mips/include/asm/compat.h | 40 +++++++++-------
arch/mips/include/uapi/asm/msgbuf.h | 57 ++++++++++++++--------
arch/mips/include/uapi/asm/sembuf.h | 15 +++++-
arch/mips/include/uapi/asm/shmbuf.h | 23 ++++++++-
arch/parisc/include/asm/compat.h | 32 ++++++-------
arch/parisc/include/uapi/asm/msgbuf.h | 33 +++++++------
arch/parisc/include/uapi/asm/sembuf.h | 16 +++----
arch/parisc/include/uapi/asm/shmbuf.h | 19 ++++----
arch/powerpc/include/asm/compat.h | 32 ++++++-------
arch/powerpc/include/uapi/asm/msgbuf.h | 18 +++----
arch/powerpc/include/uapi/asm/sembuf.h | 14 +++---
arch/powerpc/include/uapi/asm/shmbuf.h | 19 ++++----
arch/s390/include/asm/compat.h | 32 ++++++-------
arch/s390/include/uapi/asm/Kbuild | 3 ++
arch/s390/include/uapi/asm/msgbuf.h | 38 ---------------
arch/s390/include/uapi/asm/sembuf.h | 30 ------------
arch/s390/include/uapi/asm/shmbuf.h | 49 -------------------
arch/sparc/include/asm/compat.h | 32 ++++++-------
arch/sparc/include/uapi/asm/msgbuf.h | 22 ++++-----
arch/sparc/include/uapi/asm/sembuf.h | 16 +++----
arch/sparc/include/uapi/asm/shmbuf.h | 21 ++++-----
arch/x86/include/asm/compat.h | 32 ++++++-------
arch/x86/include/uapi/asm/sembuf.h | 11 ++++-
arch/xtensa/include/uapi/asm/msgbuf.h | 25 +++++-----
arch/xtensa/include/uapi/asm/sembuf.h | 17 ++++---
arch/xtensa/include/uapi/asm/shmbuf.h | 37 ++++-----------
include/linux/syscalls.h | 6 +--
include/uapi/asm-generic/msgbuf.h | 27 +++++------
include/uapi/asm-generic/sembuf.h | 26 ++++++----
include/uapi/asm-generic/shmbuf.h | 41 ++++++++--------
ipc/mqueue.c | 86 +++++++++++++++++-----------------
ipc/msg.c | 20 +++++---
ipc/sem.c | 27 +++++++----
ipc/shm.c | 14 ++++--
ipc/syscall.c | 13 +++--
ipc/util.h | 4 +-
47 files changed, 471 insertions(+), 700 deletions(-)
elete mode 100644 arch/alpha/include/uapi/asm/ipcbuf.h
delete mode 100644 arch/alpha/include/uapi/asm/msgbuf.h
delete mode 100644 arch/alpha/include/uapi/asm/sembuf.h
delete mode 100644 arch/alpha/include/uapi/asm/shmbuf.h
delete mode 100644 arch/ia64/include/uapi/asm/ipcbuf.h
delete mode 100644 arch/ia64/include/uapi/asm/msgbuf.h
delete mode 100644 arch/ia64/include/uapi/asm/sembuf.h
delete mode 100644 arch/ia64/include/uapi/asm/shmbuf.h
delete mode 100644 arch/s390/include/uapi/asm/msgbuf.h
delete mode 100644 arch/s390/include/uapi/asm/sembuf.h
delete mode 100644 arch/s390/include/uapi/asm/shmbuf.h
--
2.9.0
^ permalink raw reply
* Re: [PATCH 1/3] mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES
From: Roman Gushchin @ 2018-04-25 12:52 UTC (permalink / raw)
To: Vijayanand Jitta
Cc: vinayak menon, Vlastimil Babka, linux-mm, Andrew Morton,
Alexander Viro, Michal Hocko, Johannes Weiner, linux-fsdevel,
linux-kernel, kernel-team, Linux API
In-Reply-To: <69b4dcd8-1925-e0e8-d9b4-776f3405b769@codeaurora.org>
On Wed, Apr 25, 2018 at 09:19:29AM +0530, Vijayanand Jitta wrote:
> >>>> Idk, I don't like the idea of adding a counter outside of the vm counters
> >>>> infrastructure, and I definitely wouldn't touch the exposed
> >>>> nr_slab_reclaimable and nr_slab_unreclaimable fields.
> >>>
> >>> We would be just making the reported values more precise wrt reality.
> >>
> >> It depends on if we believe that only slab memory can be reclaimable
> >> or not. If yes, this is true, otherwise not.
> >>
> >> My guess is that some drivers (e.g. networking) might have buffers,
> >> which are reclaimable under mempressure, and are allocated using
> >> the page allocator. But I have to look closer...
> >>
> >
> > One such case I have encountered is that of the ION page pool. The page pool
> > registers a shrinker. When not in any memory pressure page pool can go high
> > and thus cause an mmap to fail when OVERCOMMIT_GUESS is set. I can send
> > a patch to account ION page pool pages in NR_INDIRECTLY_RECLAIMABLE_BYTES.
Perfect!
This is exactly what I've expected.
> >
> > Thanks,
> > Vinayak
> >
>
> As Vinayak mentioned NR_INDIRECTLY_RECLAIMABLE_BYTES can be used to solve the issue
> with ION page pool when OVERCOMMIT_GUESS is set, the patch for the same can be
> found here https://lkml.org/lkml/2018/4/24/1288
This makes perfect sense to me.
Please, fell free to add:
Acked-by: Roman Gushchin <guro@fb.com>
Thank you!
^ permalink raw reply
* Re: [REVIEW][PATCH 04/17] signal/nds32: Use force_sig in unhandled_interruption and unhandled_exceptions
From: Vincent Chen @ 2018-04-25 12:14 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, Linux Kernel Mailing List, linux-api, Vincent Chen,
Greentime Hu, Arnd Bergmann
In-Reply-To: <20180420010408.24438-4-ebiederm@xmission.com>
2018-04-20 9:03 GMT+08:00 Eric W. Biederman <ebiederm@xmission.com>:
> Neither unhandled_interrupt nor unhandled_exceptions fills in any of the
> siginfo fields whend sending SIGKILL. Further because it is SIGKILL
> even if all of the fields were filled out appropriately it would be impossible
> for the process to read any of the siginfo fields. So simplfy things and
> just use force_sig instead of force_sig_info.
>
> Fixes: 2923f5ea7738 ("nds32: Exception handling")
> Cc: Vincent Chen <vincentc@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> arch/nds32/kernel/traps.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
> index 6e34eb9824a4..65961bf91d64 100644
> --- a/arch/nds32/kernel/traps.c
> +++ b/arch/nds32/kernel/traps.c
> @@ -318,29 +318,22 @@ void do_debug_trap(unsigned long entry, unsigned long addr,
>
> void unhandled_interruption(struct pt_regs *regs)
> {
> - siginfo_t si;
> pr_emerg("unhandled_interruption\n");
> show_regs(regs);
> if (!user_mode(regs))
> do_exit(SIGKILL);
> - si.si_signo = SIGKILL;
> - si.si_errno = 0;
> - force_sig_info(SIGKILL, &si, current);
> + force_sig(SIGKILL, current);
> }
>
> void unhandled_exceptions(unsigned long entry, unsigned long addr,
> unsigned long type, struct pt_regs *regs)
> {
> - siginfo_t si;
> pr_emerg("Unhandled Exception: entry: %lx addr:%lx itype:%lx\n", entry,
> addr, type);
> show_regs(regs);
> if (!user_mode(regs))
> do_exit(SIGKILL);
> - si.si_signo = SIGKILL;
> - si.si_errno = 0;
> - si.si_addr = (void *)addr;
> - force_sig_info(SIGKILL, &si, current);
> + force_sig(SIGKILL, current);
> }
>
> extern int do_page_fault(unsigned long entry, unsigned long addr,
> --
> 2.14.1
>
Acked-by Vincent Chen
^ permalink raw reply
* Re: [REVIEW][PATCH 05/17] signal/nds32: Use force_sig(SIGILL) in do_revisn
From: Vincent Chen @ 2018-04-25 12:10 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-arch, Linux Kernel Mailing List, linux-api, Vincent Chen,
Greentime Hu, Arnd Bergmann
In-Reply-To: <20180420010408.24438-5-ebiederm@xmission.com>
2018-04-20 9:03 GMT+08:00 Eric W. Biederman <ebiederm@xmission.com>:
> As originally committed do_revisn would deliver a siginfo for SIGILL
> with an si_code composed of random stack contents. That makes no
> sense and is not something userspace can depend on. So simplify
> the code and just use "force_sig(SIG_ILL, current)" instead.
>
> Fixes: 2923f5ea7738 ("nds32: Exception handling")
> Cc: Vincent Chen <vincentc@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> arch/nds32/kernel/traps.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
> index 65961bf91d64..8e9a5b1f6234 100644
> --- a/arch/nds32/kernel/traps.c
> +++ b/arch/nds32/kernel/traps.c
> @@ -356,14 +356,11 @@ void do_dispatch_tlb_misc(unsigned long entry, unsigned long addr,
>
> void do_revinsn(struct pt_regs *regs)
> {
> - siginfo_t si;
> pr_emerg("Reserved Instruction\n");
> show_regs(regs);
> if (!user_mode(regs))
> do_exit(SIGILL);
> - si.si_signo = SIGILL;
> - si.si_errno = 0;
> - force_sig_info(SIGILL, &si, current);
> + force_sig(SIGILL, current);
> }
>
> #ifdef CONFIG_ALIGNMENT_TRAP
> --
> 2.14.1
>
You are right we do not store any valuable information in sigingo_t now.
However, We hope more information can be recorded by force_sig_fault()
as below instead of calling force_sig() directly.
- force_sig(SIGILL, current);
+ force_sig_fault(SIGILL, ILL_ILLTRP,
+ (void __user *)instruction_pointer(regs), current);
Thanks
Vincent Chen
^ permalink raw reply
* Re: [PATCH RFC v5] pidns: introduce syscall translate_pid
From: Konstantin Khlebnikov @ 2018-04-25 5:36 UTC (permalink / raw)
To: Nagarathnam Muthusamy, Eric W. Biederman
Cc: linux-api, linux-kernel, Jann Horn, Serge Hallyn, Oleg Nesterov,
Andy Lutomirski, Prakash Sangappa, Andrew Morton
In-Reply-To: <e1402871-77a5-6a0f-a75c-ccad77b93f49@oracle.com>
On 23.04.2018 20:37, Nagarathnam Muthusamy wrote:
>
>
> On 04/05/2018 12:02 AM, Konstantin Khlebnikov wrote:
>> On 05.04.2018 01:29, Eric W. Biederman wrote:
>>> Nagarathnam Muthusamy <nagarathnam.muthusamy@oracle.com> writes:
>>>
>>>> On 04/04/2018 12:11 PM, Konstantin Khlebnikov wrote:
>>>>> Each process have different pids, one for each pid namespace it belongs.
>>>>> When interaction happens within single pid-ns translation isn't required.
>>>>> More complicated scenarios needs special handling.
>>>>>
>>>>> For example:
>>>>> - reading pid-files or logs written inside container with pid namespace
>>>>> - attaching with ptrace to tasks from different pid namespace
>>>>> - passing pids across pid namespaces in any kind of API
>>>>>
>>>>> Currently there are several interfaces that could be used here:
>>>>>
>>>>> Pid namespaces are identified by inode number of /proc/[pid]/ns/pid.
>>>
>>> Using the inode number in interfaces is not an option. Especially not
>>> withou referencing the device number for the filesystem as well.
>>
>> This is supposed to be single-instance fs,
>> not part of proc but referenced but its magic "symlinks".
>>
>> Device numbers are not mentioned in "man namespaces".
>>
>>>
>>>>> Pids for nested Pid namespaces are shown in file /proc/[pid]/status.
>>>>> In some cases conversion pid -> vpid could be easily done using this
>>>>> information, but backward translation requires scanning all tasks.
>>>>>
>>>>> Unix socket automatically translates pid attached to SCM_CREDENTIALS.
>>>>> This requires CAP_SYS_ADMIN for sending arbitrary pids and entering
>>>>> into pid namespace, this expose process and could be insecure.
>>>>>
>>>>> This patch adds new syscall for converting pids between pid namespaces:
>>>>>
>>>>> pid_t translate_pid(pid_t pid, int source_type, int source,
>>>>> int target_type, int target);
>>>>>
>>>>> @source_type and @target_type defines type of following arguments:
>>>>>
>>>>> TRANSLATE_PID_CURRENT_PIDNS - current pid namespace, argument is unused
>>>>> TRANSLATE_PID_TASK_PIDNS - task pid-ns, argument is task pid
>>>>
>>>> I believe using pid to represent the namespace has been already
>>>> discussed in V1 of this patch in https://lkml.org/lkml/2015/9/22/1087
>>>> after which we moved on to fd based version of this interface.
>>>
>>> Or in short why is the case of pids important?
>>>
>>> You Konstantin you almost said why they were important in your message
>>> saying you were going to send this one. However you don't explain in
>>> your description why you want to identify pid namespaces by pid.
>>>
>>
>> Open of /proc/[pid]/ns/pid requires same permissions as ptrace,
>> pid based variant doesn't have such restrictions.
>
> Can you provide more information on usecase requiring PID translation but not used for tracing related purposes?
Any introspection for [nested] containers. It's easier to work when you have all information when you don't have any.
For example our CMS https://github.com/yandex/porto allows to start nested sub-container (or even deeper) by request from any container and
have to tell back which pid task is have. And it could translate any pid inside into accessible by client and vice versa.
> On a side note, can we have the types TRANSLATE_PID_CURRENT_PIDNS and TRANSLATE_PID_FD_PIDNS integrated first and then possibly extend the
> interface to include TRANSLATE_PID_TASK_PIDNS in future?
I don't see reason for this separation.
Pids and pid namespaces are part of the API for a long time.
>
> Thanks,
> Nagarathnam.
>> Most pid-based syscalls are racy in some cases but they are
>> here for decades and everybody knowns how to deal with it.
>> So, I've decided to merge both worlds in one interface which clearly tells what to expect.
>
^ permalink raw reply
* Re: [PATCH 1/3] mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES
From: Vijayanand Jitta @ 2018-04-25 3:49 UTC (permalink / raw)
To: vinayak menon, Roman Gushchin
Cc: Vlastimil Babka, linux-mm, Andrew Morton, Alexander Viro,
Michal Hocko, Johannes Weiner, linux-fsdevel, linux-kernel,
kernel-team, Linux API
In-Reply-To: <CAOaiJ-=JtFWNPqdtf+5uim0-LcPE9zSDZmocAa_6K3yGpW2fCQ@mail.gmail.com>
On 4/13/2018 5:43 PM, vinayak menon wrote:
> On Thu, Apr 12, 2018 at 8:27 PM, Roman Gushchin <guro@fb.com> wrote:
>> On Thu, Apr 12, 2018 at 08:52:52AM +0200, Vlastimil Babka wrote:
>>> On 04/11/2018 03:56 PM, Roman Gushchin wrote:
>>>> On Wed, Apr 11, 2018 at 03:16:08PM +0200, Vlastimil Babka wrote:
>>>>> [+CC linux-api]
>>>>>
>>>>> On 03/05/2018 02:37 PM, Roman Gushchin wrote:
>>>>>> This patch introduces a concept of indirectly reclaimable memory
>>>>>> and adds the corresponding memory counter and /proc/vmstat item.
>>>>>>
>>>>>> Indirectly reclaimable memory is any sort of memory, used by
>>>>>> the kernel (except of reclaimable slabs), which is actually
>>>>>> reclaimable, i.e. will be released under memory pressure.
>>>>>>
>>>>>> The counter is in bytes, as it's not always possible to
>>>>>> count such objects in pages. The name contains BYTES
>>>>>> by analogy to NR_KERNEL_STACK_KB.
>>>>>>
>>>>>> Signed-off-by: Roman Gushchin <guro@fb.com>
>>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>>> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
>>>>>> Cc: Michal Hocko <mhocko@suse.com>
>>>>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>>>>> Cc: linux-fsdevel@vger.kernel.org
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: linux-mm@kvack.org
>>>>>> Cc: kernel-team@fb.com
>>>>>
>>>>> Hmm, looks like I'm late and this user-visible API change was just
>>>>> merged. But it's for rc1, so we can still change it, hopefully?
>>>>>
>>>>> One problem I see with the counter is that it's in bytes, but among
>>>>> counters that use pages, and the name doesn't indicate it.
>>>>
>>>> Here I just followed "nr_kernel_stack" path, which is measured in kB,
>>>> but this is not mentioned in the field name.
>>>
>>> Oh, didn't know. Bad example to follow :P
>>>
>>>>> Then, I don't
>>>>> see why users should care about the "indirectly" part, as that's just an
>>>>> implementation detail. It is reclaimable and that's what matters, right?
>>>>> (I also wanted to complain about lack of Documentation/... update, but
>>>>> looks like there's no general file about vmstat, ugh)
>>>>
>>>> I agree, that it's a bit weird, and it's probably better to not expose
>>>> it at all; but this is how all vm counters work. We do expose them all
>>>> in /proc/vmstat. A good number of them is useless until you are not a
>>>> mm developer, so it's arguable more "debug info" rather than "api".
>>>
>>> Yeah the problem is that once tools start rely on them, they fall under
>>> the "do not break userspace" rule, however we call them. So being
>>> cautious and conservative can't hurt.
>>>
>>>> It's definitely not a reason to make them messy.
>>>> Does "nr_indirectly_reclaimable_bytes" look better to you?
>>>
>>> It still has has the "indirecly" part and feels arbitrary :/
>>>
>>>>>
>>>>> I also kind of liked the idea from v1 rfc posting that there would be a
>>>>> separate set of reclaimable kmalloc-X caches for these kind of
>>>>> allocations. Besides accounting, it should also help reduce memory
>>>>> fragmentation. The right variant of cache would be detected via
>>>>> __GFP_RECLAIMABLE.
>>>>
>>>> Well, the downside is that we have to introduce X new caches
>>>> just for this particular problem. I'm not strictly against the idea,
>>>> but not convinced that it's much better.
>>>
>>> Maybe we can find more cases that would benefit from it. Heck, even slab
>>> itself allocates some management structures from the generic kmalloc
>>> caches, and if they are used for reclaimable caches, they could be
>>> tracked as reclaimable as well.
>>
>> This is a good catch!
>>
>>>
>>>>>
>>>>> With that in mind, can we at least for now put the (manually maintained)
>>>>> byte counter in a variable that's not directly exposed via /proc/vmstat,
>>>>> and then when printing nr_slab_reclaimable, simply add the value
>>>>> (divided by PAGE_SIZE), and when printing nr_slab_unreclaimable,
>>>>> subtract the same value. This way we would be simply making the existing
>>>>> counters more precise, in line with their semantics.
>>>>
>>>> Idk, I don't like the idea of adding a counter outside of the vm counters
>>>> infrastructure, and I definitely wouldn't touch the exposed
>>>> nr_slab_reclaimable and nr_slab_unreclaimable fields.
>>>
>>> We would be just making the reported values more precise wrt reality.
>>
>> It depends on if we believe that only slab memory can be reclaimable
>> or not. If yes, this is true, otherwise not.
>>
>> My guess is that some drivers (e.g. networking) might have buffers,
>> which are reclaimable under mempressure, and are allocated using
>> the page allocator. But I have to look closer...
>>
>
> One such case I have encountered is that of the ION page pool. The page pool
> registers a shrinker. When not in any memory pressure page pool can go high
> and thus cause an mmap to fail when OVERCOMMIT_GUESS is set. I can send
> a patch to account ION page pool pages in NR_INDIRECTLY_RECLAIMABLE_BYTES.
>
> Thanks,
> Vinayak
>
As Vinayak mentioned NR_INDIRECTLY_RECLAIMABLE_BYTES can be used to solve the issue
with ION page pool when OVERCOMMIT_GUESS is set, the patch for the same can be
found here https://lkml.org/lkml/2018/4/24/1288
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Richard Guy Briggs @ 2018-04-25 0:40 UTC (permalink / raw)
To: Paul Moore
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <CAHC9VhSZd7V9avx6K5g6CQ7mkj1T8ti7Nqq=OoWVwPznkesD1w@mail.gmail.com>
On 2018-04-24 15:01, Paul Moore wrote:
> On Mon, Apr 23, 2018 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-04-23 19:15, Paul Moore wrote:
> >> On Sat, Apr 21, 2018 at 10:34 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > On 2018-04-18 19:47, Paul Moore wrote:
> >> >> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> >> > Implement the proc fs write to set the audit container ID of a process,
> >> >> > emitting an AUDIT_CONTAINER record to document the event.
> >> >> >
> >> >> > This is a write from the container orchestrator task to a proc entry of
> >> >> > the form /proc/PID/containerid where PID is the process ID of the newly
> >> >> > created task that is to become the first task in a container, or an
> >> >> > additional task added to a container.
> >> >> >
> >> >> > The write expects up to a u64 value (unset: 18446744073709551615).
> >> >> >
> >> >> > This will produce a record such as this:
> >> >> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> >> >> >
> >> >> > The "op" field indicates an initial set. The "pid" to "ses" fields are
> >> >> > the orchestrator while the "opid" field is the object's PID, the process
> >> >> > being "contained". Old and new container ID values are given in the
> >> >> > "contid" fields, while res indicates its success.
> >> >> >
> >> >> > It is not permitted to self-set, unset or re-set the container ID. A
> >> >> > child inherits its parent's container ID, but then can be set only once
> >> >> > after.
> >> >> >
> >> >> > See: https://github.com/linux-audit/audit-kernel/issues/32
> >> >> >
> >> >> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> >> >> > ---
> >> >> > fs/proc/base.c | 37 ++++++++++++++++++++
> >> >> > include/linux/audit.h | 16 +++++++++
> >> >> > include/linux/init_task.h | 4 ++-
> >> >> > include/linux/sched.h | 1 +
> >> >> > include/uapi/linux/audit.h | 2 ++
> >> >> > kernel/auditsc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
> >> >> > 6 files changed, 143 insertions(+), 1 deletion(-)
>
> ...
>
> >> >> > /* audit_rule_data supports filter rules with both integer and string
> >> >> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> >> >> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> >> >> > index 4e0a4ac..29c8482 100644
> >> >> > --- a/kernel/auditsc.c
> >> >> > +++ b/kernel/auditsc.c
> >> >> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
> >> >> > return rc;
> >> >> > }
> >> >> >
> >> >> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> >> >> > +{
> >> >> > + struct task_struct *parent;
> >> >> > + u64 pcontainerid, ccontainerid;
> >> >> > +
> >> >> > + /* Don't allow to set our own containerid */
> >> >> > + if (current == task)
> >> >> > + return -EPERM;
> >> >>
> >> >> Why not? Is there some obvious security concern that I missing?
> >> >
> >> > We then lose the distinction in the AUDIT_CONTAINER record between the
> >> > initiating PID and the target PID. This was outlined in the proposal.
> >>
> >> I just went back and reread the v3 proposal and I still don't see a
> >> good explanation of this. Why is this bad? What's the security
> >> concern?
> >
> > I don't remember, specifically. Maybe this has been addressed by the
> > check for children/threads or identical parent container ID. So, I'm
> > reluctantly willing to remove that check for now.
>
> Okay. For the record, if someone can explain to me why this
> restriction saves us from some terrible situation I'm all for leaving
> it. I'm just opposed to restrictions without solid reasoning behind
> them.
>
> >> > Having said that, I'm still not sure we have protected sufficiently from
> >> > a child turning around and setting it's parent's as yet unset or
> >> > inherited audit container ID.
> >>
> >> Yes, I believe we only want to let a task set the audit container for
> >> it's children (or itself/threads if we decide to allow that, see
> >> above). There *has* to be a function to check to see if a task if a
> >> child of a given task ... right? ... although this is likely to be a
> >> pointer traversal and locking nightmare ... hmmm.
> >
> > Isn't that just (struct task_struct)parent == (struct
> > task_struct)child->parent (or ->real_parent)?
> >
> > And now that I say that, it is covered by the following patch's child
> > check, so as long as we keep that, we should be fine.
>
> I was thinking of checking not just current's immediate children, but
> any of it's descendants as I believe that is what we want to limit,
> yes? I just worry that it isn't really practical to perform that
> check.
The child check I'm talking about prevents setting a task's audit
container ID if it *has* any children or threads, so if it has children
it is automatically disqualified and its grandchildren are irrelevant.
> >> >> I ask because I suppose it might be possible for some container
> >> >> runtime to do a fork, setup some of the environment and them exec the
> >> >> container (before you answer the obvious "namespaces!" please remember
> >> >> we're not trying to define containers).
> >> >
> >> > I don't think namespaces have any bearing on this concern since none are
> >> > required.
> >> >
> >> >> > + /* Don't allow the containerid to be unset */
> >> >> > + if (!cid_valid(containerid))
> >> >> > + return -EINVAL;
> >> >> > + /* if we don't have caps, reject */
> >> >> > + if (!capable(CAP_AUDIT_CONTROL))
> >> >> > + return -EPERM;
> >> >> > + /* if containerid is unset, allow */
> >> >> > + if (!audit_containerid_set(task))
> >> >> > + return 0;
> >> >> > + /* it is already set, and not inherited from the parent, reject */
> >> >> > + ccontainerid = audit_get_containerid(task);
> >> >> > + rcu_read_lock();
> >> >> > + parent = rcu_dereference(task->real_parent);
> >> >> > + rcu_read_unlock();
> >> >> > + task_lock(parent);
> >> >> > + pcontainerid = audit_get_containerid(parent);
> >> >> > + task_unlock(parent);
> >> >> > + if (ccontainerid != pcontainerid)
> >> >> > + return -EPERM;
> >> >> > + return 0;
>
> I'm looking at the parent checks again and I wonder if the logic above
> is what we really want. Maybe it is, but I'm not sure.
>
> Things I'm wondering about:
>
> * "ccontainerid" and "containerid" are too close in name, I kept
> confusing myself when looking at this code. Please change one. Bonus
> points if it is shorter.
Would c_containerid and p_containerid be ok? child_cid and parent_cid?
I'd really like it to have the same root as the parameter handed in so
teh code is easier to follow. It would be nice to have that across
caller to local, but that's challenging.
I've been tempted to use contid or even cid everywhere instead of
containerid. Perhaps the longer name doesn't bother me because I
like its uniqueness and I learned touch-typing in grade 9 and I like
100+ character wide terminals? ;-)
> * What if the orchestrator wants to move the task to a new container?
> Right now it looks like you can only do that once, then then the
> task's audit container ID will no longer be the same as real_parent
A task's audit container ID can be unset or inherited, and then set
only once. After that, if you want it moved to a new container you
can't and your only option is to spawn another peer to that task or a
child of it and set that new task's audit container ID.
Currently, the method of detecting if its audit container ID has been
set (rather than inherited) was to check its parent's audit container
ID. The only reason to change this might be if the audit container ID
were not inheritable, but then we lose the accountability of a task
spawning another process and being able to leave its child's audit
container ID unset and unaccountable to any existing container. I think
the relationship to the parent is crucial, and if something wants to
change audit container ID it can, by spawning childrent and leaving a
trail of container IDs in its parent processes. (So what if a parent
dies?)
> ... or does the orchestrator change that? *Can* the orchestrator
> change real_parent (I suspect the answer is "no")?
I don't think the orchestrator is able to change real_parent. I've
forgotten why there is a ->parent and ->real_parent and how they can
change. One is for the wait signal. I don't remember the purpose of
the other.
If the parent dies before the child, the child will be re-parented on
its grandparent if the parent doesn't hang around zombified, if I
understand correctly. If anything, a parent dying would likely further
restrict the ability to set a task's audit container ID because a parent
with an identical ID could vanish.
> * I think the key is the relationship between current and task, not
> between task and task->real_parent. I believe what we really care
> about is that task is a descendant of current. We might also want to
> allow current to change the audit container ID if it holds
> CAP_AUDIT_CONTROL, regardless of it's relationship with task.
Currently, a process with CAP_AUDIT_CONTROL can set the audit container
ID of any task that hasn't got children or threads, isn't itself, and
its audit container ID is inherited or unset. This was to try to
prevent games with parents and children scratching each other's backs.
I would feel more comfortable if only descendants were settable, so
adding that restriction sounds like a good idea to me other than the
tree-climbing excercise and overhead involved.
> >> >> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
> >> >> > + u64 containerid, int rc)
> >> >> > +{
> >> >> > + struct audit_buffer *ab;
> >> >> > + uid_t uid;
> >> >> > + struct tty_struct *tty;
> >> >> > +
> >> >> > + if (!audit_enabled)
> >> >> > + return;
> >> >> > +
> >> >> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> >> >> > + if (!ab)
> >> >> > + return;
> >> >> > +
> >> >> > + uid = from_kuid(&init_user_ns, task_uid(current));
> >> >> > + tty = audit_get_tty(current);
> >> >> > +
> >> >> > + audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
> >> >> > + audit_log_task_context(ab);
> >> >> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
> >> >> > + from_kuid(&init_user_ns, audit_get_loginuid(current)),
> >> >> > + tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
> >> >> > + task_tgid_nr(task), oldcontainerid, containerid, !rc);
> >> >> > +
> >> >> > + audit_put_tty(tty);
> >> >> > + audit_log_end(ab);
> >> >> > +}
> >> >> > +
> >> >> > +/**
> >> >> > + * audit_set_containerid - set current task's audit_context containerid
> >> >> > + * @containerid: containerid value
> >> >> > + *
> >> >> > + * Returns 0 on success, -EPERM on permission failure.
> >> >> > + *
> >> >> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
> >> >> > + */
> >> >> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
> >> >> > +{
> >> >> > + u64 oldcontainerid;
> >> >> > + int rc;
> >> >> > +
> >> >> > + oldcontainerid = audit_get_containerid(task);
> >> >> > +
> >> >> > + rc = audit_set_containerid_perm(task, containerid);
> >> >> > + if (!rc) {
> >> >> > + task_lock(task);
> >> >> > + task->containerid = containerid;
> >> >> > + task_unlock(task);
> >> >> > + }
> >> >> > +
> >> >> > + audit_log_set_containerid(task, oldcontainerid, containerid, rc);
> >> >> > + return rc;
> >> >>
> >> >> Why are audit_set_containerid_perm() and audit_log_containerid()
> >> >> separate functions?
> >> >
> >> > (I assume you mean audit_log_set_containerid()?)
> >>
> >> Yep. My fingers got tired typing in that function name and decided a
> >> shortcut was necessary.
> >>
> >> > It seemed clearer that all the permission checking was in one function
> >> > and its return code could be used to report the outcome when logging the
> >> > (attempted) action. This is the same structure as audit_set_loginuid()
> >> > and it made sense.
> >>
> >> When possible I really like it when the permission checks are in the
> >> same function as the code which does the work; it's less likely to get
> >> abused that way (you have to willfully bypass the access checks). The
> >> exceptions might be if you wanted to reuse the access control code, or
> >> insert a modular access mechanism (e.g. LSMs).
> >
> > I don't follow how it could be abused. The return code from the perm
> > check gates setting the value and is used in the success field in the
> > log.
>
> If the permission checks are in the same function body as the code
> which does the work you have to either split the function, or rewrite
> it, if you want to bypass the permission checks. It may be more of a
> style issue than an actual safety issue, but the comments about
> single-use functions in the same scope is the tie breaker.
Perhaps I'm just being quite dense, but I just don't follow what the
problem is and how you suggest fixing it. A bunch of gotos to a label
such as "out:" to log the refused action? That seems messy and
unstructured.
> paul moore
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH 2/7] aio: remove an outdated BUG_ON and comment in aio_complete
From: Darrick J. Wong @ 2018-04-24 21:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180415150108.1341-3-hch@lst.de>
On Sun, Apr 15, 2018 at 05:01:03PM +0200, Christoph Hellwig wrote:
> These days we don't treat sync iocbs special in the aio completion code as
> they never use it. Remove the old comment and BUG_ON given that the
> current definition of is_sync_kiocb makes it impossible to hit.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/aio.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index add46b06be86..7c1855afd723 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1107,15 +1107,6 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
> file_end_write(file);
> }
>
> - /*
> - * Special case handling for sync iocbs:
> - * - events go directly into the iocb for fast handling
> - * - the sync task with the iocb in its stack holds the single iocb
> - * ref, no other paths have a way to get another ref
> - * - the sync task helpfully left a reference to itself in the iocb
> - */
> - BUG_ON(is_sync_kiocb(kiocb));
> -
> if (iocb->ki_list.next) {
> unsigned long flags;
>
> --
> 2.17.0
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Paul Moore @ 2018-04-24 19:01 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <20180424020200.imonhbkwtb73luxl@madcap2.tricolour.ca>
On Mon, Apr 23, 2018 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-04-23 19:15, Paul Moore wrote:
>> On Sat, Apr 21, 2018 at 10:34 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2018-04-18 19:47, Paul Moore wrote:
>> >> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> >> > Implement the proc fs write to set the audit container ID of a process,
>> >> > emitting an AUDIT_CONTAINER record to document the event.
>> >> >
>> >> > This is a write from the container orchestrator task to a proc entry of
>> >> > the form /proc/PID/containerid where PID is the process ID of the newly
>> >> > created task that is to become the first task in a container, or an
>> >> > additional task added to a container.
>> >> >
>> >> > The write expects up to a u64 value (unset: 18446744073709551615).
>> >> >
>> >> > This will produce a record such as this:
>> >> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>> >> >
>> >> > The "op" field indicates an initial set. The "pid" to "ses" fields are
>> >> > the orchestrator while the "opid" field is the object's PID, the process
>> >> > being "contained". Old and new container ID values are given in the
>> >> > "contid" fields, while res indicates its success.
>> >> >
>> >> > It is not permitted to self-set, unset or re-set the container ID. A
>> >> > child inherits its parent's container ID, but then can be set only once
>> >> > after.
>> >> >
>> >> > See: https://github.com/linux-audit/audit-kernel/issues/32
>> >> >
>> >> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> >> > ---
>> >> > fs/proc/base.c | 37 ++++++++++++++++++++
>> >> > include/linux/audit.h | 16 +++++++++
>> >> > include/linux/init_task.h | 4 ++-
>> >> > include/linux/sched.h | 1 +
>> >> > include/uapi/linux/audit.h | 2 ++
>> >> > kernel/auditsc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
>> >> > 6 files changed, 143 insertions(+), 1 deletion(-)
...
>> >> > /* audit_rule_data supports filter rules with both integer and string
>> >> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>> >> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> >> > index 4e0a4ac..29c8482 100644
>> >> > --- a/kernel/auditsc.c
>> >> > +++ b/kernel/auditsc.c
>> >> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
>> >> > return rc;
>> >> > }
>> >> >
>> >> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
>> >> > +{
>> >> > + struct task_struct *parent;
>> >> > + u64 pcontainerid, ccontainerid;
>> >> > +
>> >> > + /* Don't allow to set our own containerid */
>> >> > + if (current == task)
>> >> > + return -EPERM;
>> >>
>> >> Why not? Is there some obvious security concern that I missing?
>> >
>> > We then lose the distinction in the AUDIT_CONTAINER record between the
>> > initiating PID and the target PID. This was outlined in the proposal.
>>
>> I just went back and reread the v3 proposal and I still don't see a
>> good explanation of this. Why is this bad? What's the security
>> concern?
>
> I don't remember, specifically. Maybe this has been addressed by the
> check for children/threads or identical parent container ID. So, I'm
> reluctantly willing to remove that check for now.
Okay. For the record, if someone can explain to me why this
restriction saves us from some terrible situation I'm all for leaving
it. I'm just opposed to restrictions without solid reasoning behind
them.
>> > Having said that, I'm still not sure we have protected sufficiently from
>> > a child turning around and setting it's parent's as yet unset or
>> > inherited audit container ID.
>>
>> Yes, I believe we only want to let a task set the audit container for
>> it's children (or itself/threads if we decide to allow that, see
>> above). There *has* to be a function to check to see if a task if a
>> child of a given task ... right? ... although this is likely to be a
>> pointer traversal and locking nightmare ... hmmm.
>
> Isn't that just (struct task_struct)parent == (struct
> task_struct)child->parent (or ->real_parent)?
>
> And now that I say that, it is covered by the following patch's child
> check, so as long as we keep that, we should be fine.
I was thinking of checking not just current's immediate children, but
any of it's descendants as I believe that is what we want to limit,
yes? I just worry that it isn't really practical to perform that
check.
>> >> I ask because I suppose it might be possible for some container
>> >> runtime to do a fork, setup some of the environment and them exec the
>> >> container (before you answer the obvious "namespaces!" please remember
>> >> we're not trying to define containers).
>> >
>> > I don't think namespaces have any bearing on this concern since none are
>> > required.
>> >
>> >> > + /* Don't allow the containerid to be unset */
>> >> > + if (!cid_valid(containerid))
>> >> > + return -EINVAL;
>> >> > + /* if we don't have caps, reject */
>> >> > + if (!capable(CAP_AUDIT_CONTROL))
>> >> > + return -EPERM;
>> >> > + /* if containerid is unset, allow */
>> >> > + if (!audit_containerid_set(task))
>> >> > + return 0;
>> >> > + /* it is already set, and not inherited from the parent, reject */
>> >> > + ccontainerid = audit_get_containerid(task);
>> >> > + rcu_read_lock();
>> >> > + parent = rcu_dereference(task->real_parent);
>> >> > + rcu_read_unlock();
>> >> > + task_lock(parent);
>> >> > + pcontainerid = audit_get_containerid(parent);
>> >> > + task_unlock(parent);
>> >> > + if (ccontainerid != pcontainerid)
>> >> > + return -EPERM;
>> >> > + return 0;
I'm looking at the parent checks again and I wonder if the logic above
is what we really want. Maybe it is, but I'm not sure.
Things I'm wondering about:
* "ccontainerid" and "containerid" are too close in name, I kept
confusing myself when looking at this code. Please change one. Bonus
points if it is shorter.
* What if the orchestrator wants to move the task to a new container?
Right now it looks like you can only do that once, then then the
task's audit container ID will no longer be the same as real_parent
... or does the orchestrator change that? *Can* the orchestrator
change real_parent (I suspect the answer is "no")?
* I think the key is the relationship between current and task, not
between task and task->real_parent. I believe what we really care
about is that task is a descendant of current. We might also want to
allow current to change the audit container ID if it holds
CAP_AUDIT_CONTROL, regardless of it's relationship with task.
>> >> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
>> >> > + u64 containerid, int rc)
>> >> > +{
>> >> > + struct audit_buffer *ab;
>> >> > + uid_t uid;
>> >> > + struct tty_struct *tty;
>> >> > +
>> >> > + if (!audit_enabled)
>> >> > + return;
>> >> > +
>> >> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
>> >> > + if (!ab)
>> >> > + return;
>> >> > +
>> >> > + uid = from_kuid(&init_user_ns, task_uid(current));
>> >> > + tty = audit_get_tty(current);
>> >> > +
>> >> > + audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
>> >> > + audit_log_task_context(ab);
>> >> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
>> >> > + from_kuid(&init_user_ns, audit_get_loginuid(current)),
>> >> > + tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
>> >> > + task_tgid_nr(task), oldcontainerid, containerid, !rc);
>> >> > +
>> >> > + audit_put_tty(tty);
>> >> > + audit_log_end(ab);
>> >> > +}
>> >> > +
>> >> > +/**
>> >> > + * audit_set_containerid - set current task's audit_context containerid
>> >> > + * @containerid: containerid value
>> >> > + *
>> >> > + * Returns 0 on success, -EPERM on permission failure.
>> >> > + *
>> >> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
>> >> > + */
>> >> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
>> >> > +{
>> >> > + u64 oldcontainerid;
>> >> > + int rc;
>> >> > +
>> >> > + oldcontainerid = audit_get_containerid(task);
>> >> > +
>> >> > + rc = audit_set_containerid_perm(task, containerid);
>> >> > + if (!rc) {
>> >> > + task_lock(task);
>> >> > + task->containerid = containerid;
>> >> > + task_unlock(task);
>> >> > + }
>> >> > +
>> >> > + audit_log_set_containerid(task, oldcontainerid, containerid, rc);
>> >> > + return rc;
>> >>
>> >> Why are audit_set_containerid_perm() and audit_log_containerid()
>> >> separate functions?
>> >
>> > (I assume you mean audit_log_set_containerid()?)
>>
>> Yep. My fingers got tired typing in that function name and decided a
>> shortcut was necessary.
>>
>> > It seemed clearer that all the permission checking was in one function
>> > and its return code could be used to report the outcome when logging the
>> > (attempted) action. This is the same structure as audit_set_loginuid()
>> > and it made sense.
>>
>> When possible I really like it when the permission checks are in the
>> same function as the code which does the work; it's less likely to get
>> abused that way (you have to willfully bypass the access checks). The
>> exceptions might be if you wanted to reuse the access control code, or
>> insert a modular access mechanism (e.g. LSMs).
>
> I don't follow how it could be abused. The return code from the perm
> check gates setting the value and is used in the success field in the
> log.
If the permission checks are in the same function body as the code
which does the work you have to either split the function, or rewrite
it, if you want to bypass the permission checks. It may be more of a
style issue than an actual safety issue, but the comments about
single-use functions in the same scope is the tie breaker.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/3] mm: add find_alloc_contig_pages() interface
From: Michal Hocko @ 2018-04-24 13:37 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, linux-api, Reinette Chatre,
Christopher Lameter, Guy Shattah, Anshuman Khandual,
Michal Nazarewicz, Vlastimil Babka, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <fc28bcb7-8f9a-e841-0fdf-8636523ebc2a@oracle.com>
On Sun 22-04-18 21:22:07, Mike Kravetz wrote:
> On 04/22/2018 05:09 PM, Michal Hocko wrote:
[...
> > Also do we want to check other usual suspects? E.g. PageReserved? And
> > generally migrateable pages if page count > 0. Or do we want to leave
> > everything to the alloc_contig_range?
>
> I think you proposed something like the above with limited checking at
> some time in the past. In my testing, allocations were more likely to
> succeed if we did limited testing here and let alloc_contig_range take
> a shot at migration/allocation. There really are two ways to approach
> this, do as much checking up front or let it be handled by alloc_contig_range.
OK, it would be great to have a comment mentioning that. The discrepancy
will just hit eyes
[...]
> Unless I am missing something, calls to alloc_contig range need to have
> a size that is a multiple of page block. This is because isolation needs
> to take place at a page block level. We can easily 'round up' and release
> excess pages.
I am not sure but can we simply leave a part of the page block behind? I
mean it might have a misleading migrate type but that shouldn't matter
much, no?
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH net-next 0/4] mm,tcp: provide mmap_hook to solve lockdep issue
From: Eric Dumazet @ 2018-04-24 4:30 UTC (permalink / raw)
To: Andy Lutomirski, Eric Dumazet
Cc: Eric Dumazet, David S . Miller, netdev, linux-kernel,
Soheil Hassas Yeganeh, linux-mm, Linux API
In-Reply-To: <CALCETrWOLU+P_jVpuOUQT2e_5ZShAP3OM0yJZMbC=pv5La9Cvg@mail.gmail.com>
On 04/23/2018 07:04 PM, Andy Lutomirski wrote:
> On Mon, Apr 23, 2018 at 2:38 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Hi Andy
>>
>> On 04/23/2018 02:14 PM, Andy Lutomirski wrote:
>
>>> I would suggest that you rework the interface a bit. First a user would call mmap() on a TCP socket, which would create an empty VMA. (It would set vm_ops to point to tcp_vm_ops or similar so that the TCP code could recognize it, but it would have no effect whatsoever on the TCP state machine. Reading the VMA would get SIGBUS.) Then a user would call a new ioctl() or setsockopt() function and pass something like:
>>
>>
>>>
>>> struct tcp_zerocopy_receive {
>>> void *address;
>>> size_t length;
>>> };
>>>
>>> The kernel would verify that [address, address+length) is entirely inside a single TCP VMA and then would do the vm_insert_range magic.
>>
>> I have no idea what is the proper API for that.
>> Where the TCP VMA(s) would be stored ?
>> In TCP socket, or MM layer ?
>
> MM layer. I haven't tested this at all, and the error handling is
> totally wrong, but I think you'd do something like:
>
> len = get_user(...);
>
> down_read(¤t->mm->mmap_sem);
>
> vma = find_vma(mm, start);
> if (!vma || vma->vm_start > start)
> return -EFAULT;
>
> /* This is buggy. You also need to check that the file is a socket.
> This is probably trivial. */
> if (vma->vm_file->private_data != sock)
> return -EINVAL;
>
> if (len > vma->vm_end - start)
> return -EFAULT; /* too big a request. */
>
> and now you'd do the vm_insert_page() dance, except that you don't
> have to abort the whole procedure if you discover that something isn't
> aligned right. Instead you'd just stop and tell the caller that you
> didn't map the full requested size. You might also need to add some
> code to charge the caller for the pages that get pinned, but that's an
> orthogonal issue.
>
> You also need to provide some way for user programs to signal that
> they're done with the page in question. MADV_DONTNEED might be
> sufficient.
>
> In the mmap() helper, you might want to restrict the mapped size to
> something reasonable. And it might be nice to hook mremap() to
> prevent user code from causing too much trouble.
>
> With my x86-writer-of-TLB-code hat on, I expect the major performance
> costs to be the generic costs of mmap() and munmap() (which only
> happen once per socket instead of once per read if you like my idea),
> the cost of a TLB miss when the data gets read (really not so bad on
> modern hardware), and the cost of the TLB invalidation when user code
> is done with the buffers. The latter is awful, especially in
> multithreaded programs. In fact, it's so bad that it might be worth
> mentioning in the documentation for this code that it just shouldn't
> be used in multithreaded processes. (Also, on non-PCID hardware,
> there's an annoying situation in which a recently-migrated thread that
> removes a mapping sends an IPI to the CPU that the thread used to be
> on. I thought I had a clever idea to get rid of that IPI once, but it
> turned out to be wrong.)
>
> Architectures like ARM that have superior TLB handling primitives will
> not be hurt as badly if this is used my a multithreaded program.
>
>>
>>
>> And I am not sure why the error handling would be better (point 4), unless we can return smaller @length than requested maybe ?
>
> Exactly. If I request 10MB mapped and only the first 9MB are aligned
> right, I still want the first 9 MB.
>
>>
>> Also how the VMA space would be accounted (point 3) when creating an empty VMA (no pages in there yet)
>
> There's nothing to account. It's the same as mapping /dev/null or
> similar -- the mm core should take care of it for you.
>
Thanks Andy, I am working on all this, and initial patch looks sane enough.
include/uapi/linux/tcp.h | 7 +
net/ipv4/tcp.c | 175 +++++++++++++++++++++++------------------------
2 files changed, 93 insertions(+), 89 deletions(-)
I will test all this before sending for review asap.
( I have not done the compat code yet, this can be done later I guess)
^ permalink raw reply
* Re: [PATCH net-next 0/4] mm,tcp: provide mmap_hook to solve lockdep issue
From: Andy Lutomirski @ 2018-04-24 2:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andy Lutomirski, Eric Dumazet, David S . Miller, netdev,
linux-kernel, Soheil Hassas Yeganeh, linux-mm, Linux API
In-Reply-To: <d5b4dc70-6f17-d2be-a519-5ebc3f812f57@gmail.com>
On Mon, Apr 23, 2018 at 2:38 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Hi Andy
>
> On 04/23/2018 02:14 PM, Andy Lutomirski wrote:
>> I would suggest that you rework the interface a bit. First a user would call mmap() on a TCP socket, which would create an empty VMA. (It would set vm_ops to point to tcp_vm_ops or similar so that the TCP code could recognize it, but it would have no effect whatsoever on the TCP state machine. Reading the VMA would get SIGBUS.) Then a user would call a new ioctl() or setsockopt() function and pass something like:
>
>
>>
>> struct tcp_zerocopy_receive {
>> void *address;
>> size_t length;
>> };
>>
>> The kernel would verify that [address, address+length) is entirely inside a single TCP VMA and then would do the vm_insert_range magic.
>
> I have no idea what is the proper API for that.
> Where the TCP VMA(s) would be stored ?
> In TCP socket, or MM layer ?
MM layer. I haven't tested this at all, and the error handling is
totally wrong, but I think you'd do something like:
len = get_user(...);
down_read(¤t->mm->mmap_sem);
vma = find_vma(mm, start);
if (!vma || vma->vm_start > start)
return -EFAULT;
/* This is buggy. You also need to check that the file is a socket.
This is probably trivial. */
if (vma->vm_file->private_data != sock)
return -EINVAL;
if (len > vma->vm_end - start)
return -EFAULT; /* too big a request. */
and now you'd do the vm_insert_page() dance, except that you don't
have to abort the whole procedure if you discover that something isn't
aligned right. Instead you'd just stop and tell the caller that you
didn't map the full requested size. You might also need to add some
code to charge the caller for the pages that get pinned, but that's an
orthogonal issue.
You also need to provide some way for user programs to signal that
they're done with the page in question. MADV_DONTNEED might be
sufficient.
In the mmap() helper, you might want to restrict the mapped size to
something reasonable. And it might be nice to hook mremap() to
prevent user code from causing too much trouble.
With my x86-writer-of-TLB-code hat on, I expect the major performance
costs to be the generic costs of mmap() and munmap() (which only
happen once per socket instead of once per read if you like my idea),
the cost of a TLB miss when the data gets read (really not so bad on
modern hardware), and the cost of the TLB invalidation when user code
is done with the buffers. The latter is awful, especially in
multithreaded programs. In fact, it's so bad that it might be worth
mentioning in the documentation for this code that it just shouldn't
be used in multithreaded processes. (Also, on non-PCID hardware,
there's an annoying situation in which a recently-migrated thread that
removes a mapping sends an IPI to the CPU that the thread used to be
on. I thought I had a clever idea to get rid of that IPI once, but it
turned out to be wrong.)
Architectures like ARM that have superior TLB handling primitives will
not be hurt as badly if this is used my a multithreaded program.
>
>
> And I am not sure why the error handling would be better (point 4), unless we can return smaller @length than requested maybe ?
Exactly. If I request 10MB mapped and only the first 9MB are aligned
right, I still want the first 9 MB.
>
> Also how the VMA space would be accounted (point 3) when creating an empty VMA (no pages in there yet)
There's nothing to account. It's the same as mapping /dev/null or
similar -- the mm core should take care of it for you.
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Richard Guy Briggs @ 2018-04-24 2:02 UTC (permalink / raw)
To: Paul Moore
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <CAHC9VhQkJBU-f-AuEnGF1BA2QW6nCJ_yr_EqBR02-1y9+XQZ5A@mail.gmail.com>
On 2018-04-23 19:15, Paul Moore wrote:
> On Sat, Apr 21, 2018 at 10:34 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-04-18 19:47, Paul Moore wrote:
> >> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > Implement the proc fs write to set the audit container ID of a process,
> >> > emitting an AUDIT_CONTAINER record to document the event.
> >> >
> >> > This is a write from the container orchestrator task to a proc entry of
> >> > the form /proc/PID/containerid where PID is the process ID of the newly
> >> > created task that is to become the first task in a container, or an
> >> > additional task added to a container.
> >> >
> >> > The write expects up to a u64 value (unset: 18446744073709551615).
> >> >
> >> > This will produce a record such as this:
> >> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> >> >
> >> > The "op" field indicates an initial set. The "pid" to "ses" fields are
> >> > the orchestrator while the "opid" field is the object's PID, the process
> >> > being "contained". Old and new container ID values are given in the
> >> > "contid" fields, while res indicates its success.
> >> >
> >> > It is not permitted to self-set, unset or re-set the container ID. A
> >> > child inherits its parent's container ID, but then can be set only once
> >> > after.
> >> >
> >> > See: https://github.com/linux-audit/audit-kernel/issues/32
> >> >
> >> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> >> > ---
> >> > fs/proc/base.c | 37 ++++++++++++++++++++
> >> > include/linux/audit.h | 16 +++++++++
> >> > include/linux/init_task.h | 4 ++-
> >> > include/linux/sched.h | 1 +
> >> > include/uapi/linux/audit.h | 2 ++
> >> > kernel/auditsc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
> >> > 6 files changed, 143 insertions(+), 1 deletion(-)
>
> ...
>
> >> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> >> > index d258826..1b82191 100644
> >> > --- a/include/linux/sched.h
> >> > +++ b/include/linux/sched.h
> >> > @@ -796,6 +796,7 @@ struct task_struct {
> >> > #ifdef CONFIG_AUDITSYSCALL
> >> > kuid_t loginuid;
> >> > unsigned int sessionid;
> >> > + u64 containerid;
> >>
> >> This one line addition to the task_struct scares me the most of
> >> anything in this patchset. Why? It's a field named "containerid" in
> >> a perhaps one of the most widely used core kernel structures; the
> >> possibilities for abuse are endless, and it's foolish to think we
> >> would ever be able to adequately police this.
> >
> > Fair enough.
> >
> >> Unfortunately, we can't add the field to audit_context as things
> >> currently stand because we don't always allocate an audit_context,
> >> it's dependent on the system's configuration, and we need to track the
> >> audit container ID for a given process, regardless of the audit
> >> configuration. Pretty much the same reason why loginuid and sessionid
> >> are located directly in task_struct now. As I stressed during the
> >> design phase, I really want to keep this as an *audit* container ID
> >> and not a general purpose kernel wide container ID. If the kernel
> >> ever grows a general purpose container ID token, I'll be the first in
> >> line to convert the audit code, but I don't want audit to be that
> >> general purpose mechanism ... audit is hated enough as-is ;)
> >
> > When would we need an audit container ID when audit is not enabled
> > enough to have an audit_context?
>
> I'm thinking of the audit_alloc() case where audit_filter_task()
> returns AUDIT_DISABLED.
Ok, so a task could be marked as filtered but its children would still
be auditable and inheriting its parent containerid (as well at its
loginuid and sessionid)...
> I believe this is the same reason why loginuid and sessionid live
> directly in the task_struct and not in the audit_context; they need to
> persist for the lifetime of the task.
Yes, probably.
> > If it is only used for audit, and audit is the only consumer, and audit
> > can only use it when it is enabled, then we can just return success to
> > any write to the proc filehandle, or not even present it. Nothing will
> > be able to know that value wasn't used.
> >
> > When are loginuid and sessionid used now when audit is not enabled (or
> > should I say, explicitly disabled)?
>
> See above. I think that should answer these questions.
Ok.
> >> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> >> > index 4e61a9e..921a71f 100644
> >> > --- a/include/uapi/linux/audit.h
> >> > +++ b/include/uapi/linux/audit.h
> >> > @@ -71,6 +71,7 @@
> >> > #define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
> >> > #define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
> >> > #define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
> >> > +#define AUDIT_CONTAINER 1020 /* Define the container id and information */
> >> >
> >> > #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly uninteresting to kernel */
> >> > #define AUDIT_USER_AVC 1107 /* We filter this differently */
> >> > @@ -465,6 +466,7 @@ struct audit_tty_status {
> >> > };
> >> >
> >> > #define AUDIT_UID_UNSET (unsigned int)-1
> >> > +#define AUDIT_CID_UNSET ((u64)-1)
> >>
> >> I think we need to decide if we want to distinguish between the "host"
> >> (e.g. init ns) and "unset". Looking at this patch (I've only quickly
> >> skimmed the others so far) it would appear that you don't think we
> >> need to worry about this distinction; that's fine, but let's make it
> >> explicit with a comment in the code that AUDIT_CID_UNSET means "unset"
> >> as well as "host".
> >
> > I don't see any reason to distinguish between "host" and "unset". Since
> > a container doesn't have a concrete definition based in namespaces, the
> > initial namespace set is meaningless here.
>
> Okay, that sounds reasonable.
>
> > Is there value in having a container orchestrator process have a
> > reserved container ID that has a policy distinct from any other
> > container?
>
> I'm open to arguments for this idea, but I don't see a point to it right now.
>
> > If so, then I could see the value in making the distinction.
> > For example, I've heard of interest in systemd acting as a container
> > orchestrator, so if it took on that role as PID 1, then every process in
> > the system would inherit that ID and none would be unset.
> >
> > I can't picture how having seperate "host" and "unset" values helps us.
>
> I don't have a strong feeling either way, I just wanted to ask the question.
>
> >> > /* audit_rule_data supports filter rules with both integer and string
> >> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> >> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> >> > index 4e0a4ac..29c8482 100644
> >> > --- a/kernel/auditsc.c
> >> > +++ b/kernel/auditsc.c
> >> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
> >> > return rc;
> >> > }
> >> >
> >> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> >> > +{
> >> > + struct task_struct *parent;
> >> > + u64 pcontainerid, ccontainerid;
> >> > +
> >> > + /* Don't allow to set our own containerid */
> >> > + if (current == task)
> >> > + return -EPERM;
> >>
> >> Why not? Is there some obvious security concern that I missing?
> >
> > We then lose the distinction in the AUDIT_CONTAINER record between the
> > initiating PID and the target PID. This was outlined in the proposal.
>
> I just went back and reread the v3 proposal and I still don't see a
> good explanation of this. Why is this bad? What's the security
> concern?
I don't remember, specifically. Maybe this has been addressed by the
check for children/threads or identical parent container ID. So, I'm
reluctantly willing to remove that check for now.
> > Having said that, I'm still not sure we have protected sufficiently from
> > a child turning around and setting it's parent's as yet unset or
> > inherited audit container ID.
>
> Yes, I believe we only want to let a task set the audit container for
> it's children (or itself/threads if we decide to allow that, see
> above). There *has* to be a function to check to see if a task if a
> child of a given task ... right? ... although this is likely to be a
> pointer traversal and locking nightmare ... hmmm.
Isn't that just (struct task_struct)parent == (struct
task_struct)child->parent (or ->real_parent)?
And now that I say that, it is covered by the following patch's child
check, so as long as we keep that, we should be fine.
> >> I ask because I suppose it might be possible for some container
> >> runtime to do a fork, setup some of the environment and them exec the
> >> container (before you answer the obvious "namespaces!" please remember
> >> we're not trying to define containers).
> >
> > I don't think namespaces have any bearing on this concern since none are
> > required.
> >
> >> > + /* Don't allow the containerid to be unset */
> >> > + if (!cid_valid(containerid))
> >> > + return -EINVAL;
> >> > + /* if we don't have caps, reject */
> >> > + if (!capable(CAP_AUDIT_CONTROL))
> >> > + return -EPERM;
> >> > + /* if containerid is unset, allow */
> >> > + if (!audit_containerid_set(task))
> >> > + return 0;
> >> > + /* it is already set, and not inherited from the parent, reject */
> >> > + ccontainerid = audit_get_containerid(task);
> >> > + rcu_read_lock();
> >> > + parent = rcu_dereference(task->real_parent);
> >> > + rcu_read_unlock();
> >> > + task_lock(parent);
> >> > + pcontainerid = audit_get_containerid(parent);
> >> > + task_unlock(parent);
> >> > + if (ccontainerid != pcontainerid)
> >> > + return -EPERM;
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
> >> > + u64 containerid, int rc)
> >> > +{
> >> > + struct audit_buffer *ab;
> >> > + uid_t uid;
> >> > + struct tty_struct *tty;
> >> > +
> >> > + if (!audit_enabled)
> >> > + return;
> >> > +
> >> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> >> > + if (!ab)
> >> > + return;
> >> > +
> >> > + uid = from_kuid(&init_user_ns, task_uid(current));
> >> > + tty = audit_get_tty(current);
> >> > +
> >> > + audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
> >> > + audit_log_task_context(ab);
> >> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
> >> > + from_kuid(&init_user_ns, audit_get_loginuid(current)),
> >> > + tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
> >> > + task_tgid_nr(task), oldcontainerid, containerid, !rc);
> >> > +
> >> > + audit_put_tty(tty);
> >> > + audit_log_end(ab);
> >> > +}
> >> > +
> >> > +/**
> >> > + * audit_set_containerid - set current task's audit_context containerid
> >> > + * @containerid: containerid value
> >> > + *
> >> > + * Returns 0 on success, -EPERM on permission failure.
> >> > + *
> >> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
> >> > + */
> >> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
> >> > +{
> >> > + u64 oldcontainerid;
> >> > + int rc;
> >> > +
> >> > + oldcontainerid = audit_get_containerid(task);
> >> > +
> >> > + rc = audit_set_containerid_perm(task, containerid);
> >> > + if (!rc) {
> >> > + task_lock(task);
> >> > + task->containerid = containerid;
> >> > + task_unlock(task);
> >> > + }
> >> > +
> >> > + audit_log_set_containerid(task, oldcontainerid, containerid, rc);
> >> > + return rc;
> >>
> >> Why are audit_set_containerid_perm() and audit_log_containerid()
> >> separate functions?
> >
> > (I assume you mean audit_log_set_containerid()?)
>
> Yep. My fingers got tired typing in that function name and decided a
> shortcut was necessary.
>
> > It seemed clearer that all the permission checking was in one function
> > and its return code could be used to report the outcome when logging the
> > (attempted) action. This is the same structure as audit_set_loginuid()
> > and it made sense.
>
> When possible I really like it when the permission checks are in the
> same function as the code which does the work; it's less likely to get
> abused that way (you have to willfully bypass the access checks). The
> exceptions might be if you wanted to reuse the access control code, or
> insert a modular access mechanism (e.g. LSMs).
I don't follow how it could be abused. The return code from the perm
check gates setting the value and is used in the success field in the
log.
> I'm less concerned about audit_log_set_containerid(), but the usual
> idea of avoiding single-use function within the same scope applies
> here.
>
> > This would be the time to connect it to a syscall if that seems like a
> > good idea and remove pid, uid, auid, tty, ses fields.
>
> Ah yes, I missed that. You know my stance on connecting records by
> now (hint: yes, connect them) so I think that would be a good thing to
> do for the next round.
Ok...
> paul moore
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Paul Moore @ 2018-04-23 23:15 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <20180421143443.faaput5g2rn6ul7p@madcap2.tricolour.ca>
On Sat, Apr 21, 2018 at 10:34 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-04-18 19:47, Paul Moore wrote:
>> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > Implement the proc fs write to set the audit container ID of a process,
>> > emitting an AUDIT_CONTAINER record to document the event.
>> >
>> > This is a write from the container orchestrator task to a proc entry of
>> > the form /proc/PID/containerid where PID is the process ID of the newly
>> > created task that is to become the first task in a container, or an
>> > additional task added to a container.
>> >
>> > The write expects up to a u64 value (unset: 18446744073709551615).
>> >
>> > This will produce a record such as this:
>> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>> >
>> > The "op" field indicates an initial set. The "pid" to "ses" fields are
>> > the orchestrator while the "opid" field is the object's PID, the process
>> > being "contained". Old and new container ID values are given in the
>> > "contid" fields, while res indicates its success.
>> >
>> > It is not permitted to self-set, unset or re-set the container ID. A
>> > child inherits its parent's container ID, but then can be set only once
>> > after.
>> >
>> > See: https://github.com/linux-audit/audit-kernel/issues/32
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> > fs/proc/base.c | 37 ++++++++++++++++++++
>> > include/linux/audit.h | 16 +++++++++
>> > include/linux/init_task.h | 4 ++-
>> > include/linux/sched.h | 1 +
>> > include/uapi/linux/audit.h | 2 ++
>> > kernel/auditsc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
>> > 6 files changed, 143 insertions(+), 1 deletion(-)
...
>> > diff --git a/include/linux/sched.h b/include/linux/sched.h
>> > index d258826..1b82191 100644
>> > --- a/include/linux/sched.h
>> > +++ b/include/linux/sched.h
>> > @@ -796,6 +796,7 @@ struct task_struct {
>> > #ifdef CONFIG_AUDITSYSCALL
>> > kuid_t loginuid;
>> > unsigned int sessionid;
>> > + u64 containerid;
>>
>> This one line addition to the task_struct scares me the most of
>> anything in this patchset. Why? It's a field named "containerid" in
>> a perhaps one of the most widely used core kernel structures; the
>> possibilities for abuse are endless, and it's foolish to think we
>> would ever be able to adequately police this.
>
> Fair enough.
>
>> Unfortunately, we can't add the field to audit_context as things
>> currently stand because we don't always allocate an audit_context,
>> it's dependent on the system's configuration, and we need to track the
>> audit container ID for a given process, regardless of the audit
>> configuration. Pretty much the same reason why loginuid and sessionid
>> are located directly in task_struct now. As I stressed during the
>> design phase, I really want to keep this as an *audit* container ID
>> and not a general purpose kernel wide container ID. If the kernel
>> ever grows a general purpose container ID token, I'll be the first in
>> line to convert the audit code, but I don't want audit to be that
>> general purpose mechanism ... audit is hated enough as-is ;)
>
> When would we need an audit container ID when audit is not enabled
> enough to have an audit_context?
I'm thinking of the audit_alloc() case where audit_filter_task()
returns AUDIT_DISABLED.
I believe this is the same reason why loginuid and sessionid live
directly in the task_struct and not in the audit_context; they need to
persist for the lifetime of the task.
> If it is only used for audit, and audit is the only consumer, and audit
> can only use it when it is enabled, then we can just return success to
> any write to the proc filehandle, or not even present it. Nothing will
> be able to know that value wasn't used.
>
> When are loginuid and sessionid used now when audit is not enabled (or
> should I say, explicitly disabled)?
See above. I think that should answer these questions.
>> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> > index 4e61a9e..921a71f 100644
>> > --- a/include/uapi/linux/audit.h
>> > +++ b/include/uapi/linux/audit.h
>> > @@ -71,6 +71,7 @@
>> > #define AUDIT_TTY_SET 1017 /* Set TTY auditing status */
>> > #define AUDIT_SET_FEATURE 1018 /* Turn an audit feature on or off */
>> > #define AUDIT_GET_FEATURE 1019 /* Get which features are enabled */
>> > +#define AUDIT_CONTAINER 1020 /* Define the container id and information */
>> >
>> > #define AUDIT_FIRST_USER_MSG 1100 /* Userspace messages mostly uninteresting to kernel */
>> > #define AUDIT_USER_AVC 1107 /* We filter this differently */
>> > @@ -465,6 +466,7 @@ struct audit_tty_status {
>> > };
>> >
>> > #define AUDIT_UID_UNSET (unsigned int)-1
>> > +#define AUDIT_CID_UNSET ((u64)-1)
>>
>> I think we need to decide if we want to distinguish between the "host"
>> (e.g. init ns) and "unset". Looking at this patch (I've only quickly
>> skimmed the others so far) it would appear that you don't think we
>> need to worry about this distinction; that's fine, but let's make it
>> explicit with a comment in the code that AUDIT_CID_UNSET means "unset"
>> as well as "host".
>
> I don't see any reason to distinguish between "host" and "unset". Since
> a container doesn't have a concrete definition based in namespaces, the
> initial namespace set is meaningless here.
Okay, that sounds reasonable.
> Is there value in having a container orchestrator process have a
> reserved container ID that has a policy distinct from any other
> container?
I'm open to arguments for this idea, but I don't see a point to it right now.
> If so, then I could see the value in making the distinction.
> For example, I've heard of interest in systemd acting as a container
> orchestrator, so if it took on that role as PID 1, then every process in
> the system would inherit that ID and none would be unset.
>
> I can't picture how having seperate "host" and "unset" values helps us.
I don't have a strong feeling either way, I just wanted to ask the question.
>> > /* audit_rule_data supports filter rules with both integer and string
>> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> > index 4e0a4ac..29c8482 100644
>> > --- a/kernel/auditsc.c
>> > +++ b/kernel/auditsc.c
>> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
>> > return rc;
>> > }
>> >
>> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
>> > +{
>> > + struct task_struct *parent;
>> > + u64 pcontainerid, ccontainerid;
>> > +
>> > + /* Don't allow to set our own containerid */
>> > + if (current == task)
>> > + return -EPERM;
>>
>> Why not? Is there some obvious security concern that I missing?
>
> We then lose the distinction in the AUDIT_CONTAINER record between the
> initiating PID and the target PID. This was outlined in the proposal.
I just went back and reread the v3 proposal and I still don't see a
good explanation of this. Why is this bad? What's the security
concern?
> Having said that, I'm still not sure we have protected sufficiently from
> a child turning around and setting it's parent's as yet unset or
> inherited audit container ID.
Yes, I believe we only want to let a task set the audit container for
it's children (or itself/threads if we decide to allow that, see
above). There *has* to be a function to check to see if a task if a
child of a given task ... right? ... although this is likely to be a
pointer traversal and locking nightmare ... hmmm.
>> I ask because I suppose it might be possible for some container
>> runtime to do a fork, setup some of the environment and them exec the
>> container (before you answer the obvious "namespaces!" please remember
>> we're not trying to define containers).
>
> I don't think namespaces have any bearing on this concern since none are
> required.
>
>> > + /* Don't allow the containerid to be unset */
>> > + if (!cid_valid(containerid))
>> > + return -EINVAL;
>> > + /* if we don't have caps, reject */
>> > + if (!capable(CAP_AUDIT_CONTROL))
>> > + return -EPERM;
>> > + /* if containerid is unset, allow */
>> > + if (!audit_containerid_set(task))
>> > + return 0;
>> > + /* it is already set, and not inherited from the parent, reject */
>> > + ccontainerid = audit_get_containerid(task);
>> > + rcu_read_lock();
>> > + parent = rcu_dereference(task->real_parent);
>> > + rcu_read_unlock();
>> > + task_lock(parent);
>> > + pcontainerid = audit_get_containerid(parent);
>> > + task_unlock(parent);
>> > + if (ccontainerid != pcontainerid)
>> > + return -EPERM;
>> > + return 0;
>> > +}
>> > +
>> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
>> > + u64 containerid, int rc)
>> > +{
>> > + struct audit_buffer *ab;
>> > + uid_t uid;
>> > + struct tty_struct *tty;
>> > +
>> > + if (!audit_enabled)
>> > + return;
>> > +
>> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
>> > + if (!ab)
>> > + return;
>> > +
>> > + uid = from_kuid(&init_user_ns, task_uid(current));
>> > + tty = audit_get_tty(current);
>> > +
>> > + audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
>> > + audit_log_task_context(ab);
>> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
>> > + from_kuid(&init_user_ns, audit_get_loginuid(current)),
>> > + tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
>> > + task_tgid_nr(task), oldcontainerid, containerid, !rc);
>> > +
>> > + audit_put_tty(tty);
>> > + audit_log_end(ab);
>> > +}
>> > +
>> > +/**
>> > + * audit_set_containerid - set current task's audit_context containerid
>> > + * @containerid: containerid value
>> > + *
>> > + * Returns 0 on success, -EPERM on permission failure.
>> > + *
>> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
>> > + */
>> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
>> > +{
>> > + u64 oldcontainerid;
>> > + int rc;
>> > +
>> > + oldcontainerid = audit_get_containerid(task);
>> > +
>> > + rc = audit_set_containerid_perm(task, containerid);
>> > + if (!rc) {
>> > + task_lock(task);
>> > + task->containerid = containerid;
>> > + task_unlock(task);
>> > + }
>> > +
>> > + audit_log_set_containerid(task, oldcontainerid, containerid, rc);
>> > + return rc;
>>
>> Why are audit_set_containerid_perm() and audit_log_containerid()
>> separate functions?
>
> (I assume you mean audit_log_set_containerid()?)
Yep. My fingers got tired typing in that function name and decided a
shortcut was necessary.
> It seemed clearer that all the permission checking was in one function
> and its return code could be used to report the outcome when logging the
> (attempted) action. This is the same structure as audit_set_loginuid()
> and it made sense.
When possible I really like it when the permission checks are in the
same function as the code which does the work; it's less likely to get
abused that way (you have to willfully bypass the access checks). The
exceptions might be if you wanted to reuse the access control code, or
insert a modular access mechanism (e.g. LSMs).
I'm less concerned about audit_log_set_containerid(), but the usual
idea of avoiding single-use function within the same scope applies
here.
> This would be the time to connect it to a syscall if that seems like a
> good idea and remove pid, uid, auid, tty, ses fields.
Ah yes, I missed that. You know my stance on connecting records by
now (hint: yes, connect them) so I think that would be a good thing to
do for the next round.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH net-next 0/4] mm,tcp: provide mmap_hook to solve lockdep issue
From: Eric Dumazet @ 2018-04-23 21:38 UTC (permalink / raw)
To: Andy Lutomirski, Eric Dumazet, David S . Miller
Cc: netdev, linux-kernel, Soheil Hassas Yeganeh, Eric Dumazet,
linux-mm, Linux API
In-Reply-To: <9ed6083f-d731-945c-dbcd-f977c5600b03@kernel.org>
Hi Andy
On 04/23/2018 02:14 PM, Andy Lutomirski wrote:
> On 04/20/2018 08:55 AM, Eric Dumazet wrote:
>> This patch series provide a new mmap_hook to fs willing to grab
>> a mutex before mm->mmap_sem is taken, to ensure lockdep sanity.
>>
>> This hook allows us to shorten tcp_mmap() execution time (while mmap_sem
>> is held), and improve multi-threading scalability.
>>
>
> I think that the right solution is to rework mmap() on TCP sockets a bit. The current approach in net-next is very strange for a few reasons:
>
> 1. It uses mmap() as an operation that has side effects besides just creating a mapping. If nothing else, it's surprising, since mmap() doesn't usually do that. But it's also causing problems like what you're seeing.
>
> 2. The performance is worse than it needs to be. mmap() is slow, and I doubt you'll find many mm developers who consider this particular abuse of mmap() to be a valid thing to optimize for.
>
> 3. I'm not at all convinced the accounting is sane. As far as I can tell, you're allowing unprivileged users to increment the count on network-owned pages, limited only by available virtual memory, without obviously charging it to the socket buffer limits. It looks like a program that simply forgot to call munmap() would cause the system to run out of memory, and I see no reason to expect the OOM killer to have any real chance of killing the right task.
>
> 4. Error handling sucks. If I try to mmap() a large range (which is the whole point -- using a small range will kill performance) and not quite all of it can be mapped, then I waste a bunch of time in the kernel and get *none* of the range mapped.
>
> I would suggest that you rework the interface a bit. First a user would call mmap() on a TCP socket, which would create an empty VMA. (It would set vm_ops to point to tcp_vm_ops or similar so that the TCP code could recognize it, but it would have no effect whatsoever on the TCP state machine. Reading the VMA would get SIGBUS.) Then a user would call a new ioctl() or setsockopt() function and pass something like:
>
> struct tcp_zerocopy_receive {
> void *address;
> size_t length;
> };
>
> The kernel would verify that [address, address+length) is entirely inside a single TCP VMA and then would do the vm_insert_range magic.
I have no idea what is the proper API for that.
Where the TCP VMA(s) would be stored ?
In TCP socket, or MM layer ?
And I am not sure why the error handling would be better (point 4), unless we can return smaller @length than requested maybe ?
Also how the VMA space would be accounted (point 3) when creating an empty VMA (no pages in there yet)
On success, length is changed to the length that actually got mapped. The kernel could do this while holding mmap_sem for *read*, and it could get the lock ordering right. If and when mm range locks ever get merged, it could switch to using a range lock.
>
> Then you could use MADV_DONTNEED or another ioctl/setsockopt to zap the part of the mapping that you're done with.
>
> Does this seem reasonable? It should involve very little code change, it will run faster, it will scale better, and it is much less weird IMO.
Maybe, although I do not see the 'little code change' yet.
But at least, this seems pretty nice idea, especially if it could allow us to fill the mmap()ed area later when packets are received.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox