Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
From: Mathieu Desnoyers @ 2018-07-03 18:15 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Andi Kleen, heiko carstens, Andy Lutomirski,
	Thomas Gleixner, linux-kernel, linux-api, Paul E. McKenney,
	Boqun Feng, Dave Watson, Paul Turner, Andrew Morton, Russell King,
	Ingo Molnar, H. Peter Anvin, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will
In-Reply-To: <20180703181143.GB2494@hirez.programming.kicks-ass.net>

----- On Jul 3, 2018, at 2:11 PM, Peter Zijlstra peterz@infradead.org wrote:

> On Tue, Jul 03, 2018 at 01:58:37PM -0400, Mathieu Desnoyers wrote:
>> I can modify the ABI to put the cpu_id_start and cpu_id fields inside
>> a union, and update it with a single store.
>> 
>> Thoughts ?
> 
> Let's keep them for now, we can always frob this later, they are aligned
> and proper, no need to expose that union to userspace.

Isn't it weird to change the API of an exposed public uapi header ? What
if userspace chooses to do sizeof(__rseq_abi.cpu_id) ? We would break
this unless we use a transparent union, which puts constraints I would
hope not to have on compilers supporting transparent unions (I recall
C++ had issues with this).

I'd prefer to expose the union right away if it's fine with you.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
From: Peter Zijlstra @ 2018-07-03 18:28 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Linus Torvalds, Andi Kleen, heiko carstens, Andy Lutomirski,
	Thomas Gleixner, linux-kernel, linux-api, Paul E. McKenney,
	Boqun Feng, Dave Watson, Paul Turner, Andrew Morton, Russell King,
	Ingo Molnar, H. Peter Anvin, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will
In-Reply-To: <1708848118.11868.1530641734202.JavaMail.zimbra@efficios.com>

On Tue, Jul 03, 2018 at 02:15:34PM -0400, Mathieu Desnoyers wrote:
> ----- On Jul 3, 2018, at 2:11 PM, Peter Zijlstra peterz@infradead.org wrote:
> 
> > On Tue, Jul 03, 2018 at 01:58:37PM -0400, Mathieu Desnoyers wrote:
> >> I can modify the ABI to put the cpu_id_start and cpu_id fields inside
> >> a union, and update it with a single store.
> >> 
> >> Thoughts ?
> > 
> > Let's keep them for now, we can always frob this later, they are aligned
> > and proper, no need to expose that union to userspace.
> 
> Isn't it weird to change the API of an exposed public uapi header ?

Sure, just keep it as is. We don't need an exposed union to do a single
store there.

Something like the ugly below preserves API but still does a single
store.

But sure, if you want to expose that union for some reason, then now is
the time.

diff --git a/kernel/rseq.c b/kernel/rseq.c
index 22b6acf1ad63..e956c48b5f83 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -85,10 +85,17 @@ static int rseq_update_cpu_id(struct task_struct *t)
 {
 	u32 cpu_id = raw_smp_processor_id();
 
-	if (__put_user(cpu_id, &t->rseq->cpu_id_start))
-		return -EFAULT;
-	if (__put_user(cpu_id, &t->rseq->cpu_id))
+	union {
+		struct {
+			u32 cpu_id_start;
+			u32 cpu_id;
+		};
+		u64 val;
+	} x = { { .cpu_id_start = cpu_id, .cpu_id = cpu_id, } };
+
+	if (__put_user(x.val, (u64 *)&t->rseq->cpu_id_start))
 		return -EFAULT;
+
 	trace_rseq_update(t);
 	return 0;
 }

^ permalink raw reply related

* Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
From: Mathieu Desnoyers @ 2018-07-03 18:41 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Andi Kleen, heiko carstens, Andy Lutomirski,
	Thomas Gleixner, linux-kernel, linux-api, Paul E. McKenney,
	Boqun Feng, Dave Watson, Paul Turner, Andrew Morton, Russell King,
	Ingo Molnar, H. Peter Anvin, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will
In-Reply-To: <20180703182837.GC2494@hirez.programming.kicks-ass.net>

----- On Jul 3, 2018, at 2:28 PM, Peter Zijlstra peterz@infradead.org wrote:

> On Tue, Jul 03, 2018 at 02:15:34PM -0400, Mathieu Desnoyers wrote:
>> ----- On Jul 3, 2018, at 2:11 PM, Peter Zijlstra peterz@infradead.org wrote:
>> 
>> > On Tue, Jul 03, 2018 at 01:58:37PM -0400, Mathieu Desnoyers wrote:
>> >> I can modify the ABI to put the cpu_id_start and cpu_id fields inside
>> >> a union, and update it with a single store.
>> >> 
>> >> Thoughts ?
>> > 
>> > Let's keep them for now, we can always frob this later, they are aligned
>> > and proper, no need to expose that union to userspace.
>> 
>> Isn't it weird to change the API of an exposed public uapi header ?
> 
> Sure, just keep it as is. We don't need an exposed union to do a single
> store there.
> 
> Something like the ugly below preserves API but still does a single
> store.
> 
> But sure, if you want to expose that union for some reason, then now is
> the time.

User-space won't ever want to read cpu_id_start and cpu_id from a single
u64 load, it serves no purpose to do so. So I'm OK with keeping those as
is and defining a local union for the __put_user() update.

Thanks!

Mathieu

> 
> diff --git a/kernel/rseq.c b/kernel/rseq.c
> index 22b6acf1ad63..e956c48b5f83 100644
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -85,10 +85,17 @@ static int rseq_update_cpu_id(struct task_struct *t)
> {
> 	u32 cpu_id = raw_smp_processor_id();
> 
> -	if (__put_user(cpu_id, &t->rseq->cpu_id_start))
> -		return -EFAULT;
> -	if (__put_user(cpu_id, &t->rseq->cpu_id))
> +	union {
> +		struct {
> +			u32 cpu_id_start;
> +			u32 cpu_id;
> +		};
> +		u64 val;
> +	} x = { { .cpu_id_start = cpu_id, .cpu_id = cpu_id, } };
> +
> +	if (__put_user(x.val, (u64 *)&t->rseq->cpu_id_start))
> 		return -EFAULT;
> +
> 	trace_rseq_update(t);
> 	return 0;
>  }

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
From: Peter Zijlstra @ 2018-07-03 19:08 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Linus Torvalds, Andi Kleen, heiko carstens, Andy Lutomirski,
	Thomas Gleixner, linux-kernel, linux-api, Paul E. McKenney,
	Boqun Feng, Dave Watson, Paul Turner, Andrew Morton, Russell King,
	Ingo Molnar, H. Peter Anvin, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will
In-Reply-To: <1368400582.11897.1530643261772.JavaMail.zimbra@efficios.com>

On Tue, Jul 03, 2018 at 02:41:01PM -0400, Mathieu Desnoyers wrote:

> User-space won't ever want to read cpu_id_start and cpu_id from a single
> u64 load, it serves no purpose to do so. So I'm OK with keeping those as
> is and defining a local union for the __put_user() update.

So I think previously we had the sequence number and cpuid in there
together, and in that case it did want to load them both. But since you
made that sequence number dissapear....

^ permalink raw reply

* Re: [REGRESSION] "Locked" and "Pss" in /proc/*/smaps are the same
From: Vlastimil Babka @ 2018-07-04  8:46 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Thomas Lindroth, Andrew Morton, linux-api, linux-kernel, linux-mm
In-Reply-To: <CAKOZuev9K0EMpqBoie4H7XduB63KayORxO=JEZvS9rv_4PVsqQ@mail.gmail.com>

On 07/03/2018 06:20 PM, Daniel Colascione wrote:
> On Tue, Jul 3, 2018 at 12:36 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
>> +CC
>>
>> On 07/01/2018 08:31 PM, Thomas Lindroth wrote:
>>> While looking around in /proc on my v4.14.52 system I noticed that
>>> all processes got a lot of "Locked" memory in /proc/*/smaps. A lot
>>> more memory than a regular user can usually lock with mlock().
>>>
>>> commit 493b0e9d945fa9dfe96be93ae41b4ca4b6fdb317 (v4.14-rc1) seems
>>> to have changed the behavior of "Locked".
> 
> Thanks for fixing that. I submitted a patch [1] for this bug and some
> others a while ago, but the patch didn't make it into the tree because
> or wasn't split up correctly or something, and I had to do other work.

Hmm I see. I pondered about the patch and wondered if the scenarios it
fixes are really possible for smaps_rollup. Did you observe them in
practice? Namely:
- when seq_file starts and stops multiple times on a single open file
description
- when it issues multiple show calls for the same iterator value

I don't think it can happen when all positions but the last one just
return SEQ_SKIP.

Anyway I think the seq_file iterator API usage for smaps_rollup is
unnecessary. Semantically the file shows only one "element" and that's
the set of rollup values for all vmas. Letting seq_file do the iteration
over vmas brings only complications?

> [1] https://marc.info/?l=linux-mm&m=151927723128134&w=2
> 

^ permalink raw reply

* Re: [PATCH 7/7] aio: implement io_pgetevents
From: Adrian Reber @ 2018-07-04 14:21 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
	linux-kernel
In-Reply-To: <20180502211448.18276-8-hch@lst.de>

On Wed, May 02, 2018 at 11:14:48PM +0200, Christoph Hellwig wrote:
> This is the io_getevents equivalent of ppoll/pselect and allows to
> properly mix signals and aio completions (especially with IOCB_CMD_POLL)
> and atomically executes the following sequence:
> 
> 	sigset_t origmask;
> 
> 	pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
> 	ret = io_getevents(ctx, min_nr, nr, events, timeout);
> 	pthread_sigmask(SIG_SETMASK, &origmask, NULL);
> 
> Note that unlike many other signal related calls we do not pass a sigmask
> size, as that would get us to 7 arguments, which aren't easily supported
> by the syscall infrastructure.  It seems a lot less painful to just add a
> new syscall variant in the unlikely case we're going to increase the
> sigset size.

Starting with this commit following code does not compile for me
anymore:

#include <signal.h>
#include <linux/aio_abi.h>

int main()
{
        return 0;
}

In file included from /usr/include/linux/signal.h:5,
                 from /usr/include/linux/aio_abi.h:32,
                 from include.c:2:
/usr/include/asm/signal.h:16:23: error: conflicting types for ‘sigset_t’
 typedef unsigned long sigset_t;
                       ^~~~~~~~
In file included from /usr/include/signal.h:35,
                 from include.c:1:
/usr/include/bits/types/sigset_t.h:7:20: note: previous declaration of ‘sigset_t’ was here
 typedef __sigset_t sigset_t;
                    ^~~~~~~~
In file included from /usr/include/linux/signal.h:5,
                 from /usr/include/linux/aio_abi.h:32,
                 from include.c:2:
/usr/include/asm/signal.h:115:8: error: redefinition of ‘struct sigaction’
 struct sigaction {
        ^~~~~~~~~
In file included from /usr/include/signal.h:226,
                 from include.c:1:
/usr/include/bits/sigaction.h:27:8: note: originally defined here
 struct sigaction
        ^~~~~~~~~
[and much more]

Before this commit it compiles without errors.

		Adrian

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

* Review request for glibc system call wrapper for statx
From: Florian Weimer @ 2018-07-05 17:13 UTC (permalink / raw)
  To: Linux FS Devel, Linux API; +Cc: GNU C Library, David Howells, Alexander Viro

I've proposed a statx system call wrapper for glibc:

   https://sourceware.org/ml/libc-alpha/2018-06/msg01038.html

The somewhat questionable part is the userspace emulation if the kernel 
does not support statx.  It looks like this:

+/* Approximate emulation of statx.  This will always fill in
+   POSIX-mandated attributes even if the underlying file system does
+   not actually support it (for example, GID and UID on file systems
+   without UNIX-style permissions).  */
+static __attribute__ ((unused)) int
+statx_generic (int fd, const char *path, int flags,
+               unsigned int mask, struct statx *buf)
+{
+  /* Flags which need to be cleared before passing them to
+     fstatat64.  */
+  static const int clear_flags = AT_STATX_SYNC_AS_STAT;
+
+    /* Flags supported by our emulation.  */
+  static const int supported_flags
+    = AT_EMPTY_PATH | AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW
+      | clear_flags;
+
+  if (__glibc_unlikely ((flags & ~supported_flags) != 0))
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
+  struct stat64 st;
+  int ret = __fstatat64 (fd, path, &st, flags & ~clear_flags);
+  if (ret != 0)
+    return ret;
+
+  *buf = (struct statx)
+    {
+      /* We copy everything from fstat64, which corresponds the basic
+         fstat64.  */
+      .stx_mask = STATX_BASIC_STATS,
+      .stx_blksize = st.st_blksize,
+      .stx_nlink = st.st_nlink,
+      .stx_uid = st.st_uid,
+      .stx_gid = st.st_gid,
+      .stx_mode = st.st_mode,
+      .stx_ino = st.st_ino,
+      .stx_size = st.st_size,
+      .stx_blocks = st.st_blocks,
+      .stx_atime = statx_convert_timestamp (st.st_atim),
+      .stx_ctime = statx_convert_timestamp (st.st_ctim),
+      .stx_mtime = statx_convert_timestamp (st.st_mtim),
+      .stx_rdev_major = __gnu_dev_major (st.st_rdev),
+      .stx_rdev_minor = __gnu_dev_minor (st.st_rdev),
+      .stx_dev_major = __gnu_dev_minor (st.st_dev),
+      .stx_dev_minor = __gnu_dev_minor (st.st_dev),
+    };
+
+  return 0;
+}

Do you think this emulation is a good idea?  Or should we drop it and 
just return ENOSYS?

Thanks,
Florian

^ permalink raw reply

* [RFC PATCH for 4.18 0/5] Restartable Sequences updates
From: Mathieu Desnoyers @ 2018-07-05 18:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk

Following the recent discussion thread [1] about rseq uapi, here is
a set of updates submitted for comments.

Thanks,

Mathieu

[1] https://lkml.kernel.org/r/20180702223143.4663-1-mathieu.desnoyers@efficios.com

Mathieu Desnoyers (5):
  rseq: use __u64 for rseq_cs fields, validate user inputs
  rseq: uapi: update uapi comments
  rseq: uapi: declare rseq_cs field as union, update includes
  rseq: remove unused types_32_64.h uapi header
  rseq/selftests: cleanup: update comment above rseq_prepare_unload

 include/uapi/linux/rseq.h           | 102 ++++++++++++++++++++----------------
 include/uapi/linux/types_32_64.h    |  50 ------------------
 kernel/rseq.c                       |  26 +++++----
 tools/testing/selftests/rseq/rseq.h |  24 ++++++---
 4 files changed, 92 insertions(+), 110 deletions(-)
 delete mode 100644 include/uapi/linux/types_32_64.h

-- 
2.11.0

^ permalink raw reply

* [RFC PATCH for 4.18 1/5] rseq: use __u64 for rseq_cs fields, validate user inputs
From: Mathieu Desnoyers @ 2018-07-05 18:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20180705180601.18423-1-mathieu.desnoyers@efficios.com>

Change the rseq ABI so rseq_cs start_ip, post_commit_offset and abort_ip
fields are seen as 64-bit fields by both 32-bit and 64-bit kernels rather
that ignoring the 32 upper bits on 32-bit kernels. This ensures we have a
consistent behavior for a 32-bit binary executed on 32-bit kernels and in
compat mode on 64-bit kernels.

Validating the value of abort_ip field to be below TASK_SIZE ensures the
kernel don't return to an invalid address when returning to userspace
after an abort. I don't fully trust each architecture code to consistently
deal with invalid return addresses.

Validating the value of the start_ip and post_commit_offset fields
prevents overflow on arithmetic performed on those values, used to
check whether abort_ip is within the rseq critical section.

If validation fails, the process is killed with a segmentation fault.

When the signature encountered before abort_ip does not match the expected
signature, return -EINVAL rather than -EPERM to be consistent with other
input validation return codes from rseq_get_rseq_cs().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 include/uapi/linux/rseq.h |  6 +++---
 kernel/rseq.c             | 14 ++++++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index d620fa43756c..519ad6e176d1 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -52,10 +52,10 @@ struct rseq_cs {
 	__u32 version;
 	/* enum rseq_cs_flags */
 	__u32 flags;
-	LINUX_FIELD_u32_u64(start_ip);
+	__u64 start_ip;
 	/* Offset from start_ip. */
-	LINUX_FIELD_u32_u64(post_commit_offset);
-	LINUX_FIELD_u32_u64(abort_ip);
+	__u64 post_commit_offset;
+	__u64 abort_ip;
 } __attribute__((aligned(4 * sizeof(__u64))));
 
 /*
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 22b6acf1ad63..16b38c5342f9 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -130,14 +130,20 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
 	urseq_cs = (struct rseq_cs __user *)ptr;
 	if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
 		return -EFAULT;
-	if (rseq_cs->version > 0)
-		return -EINVAL;
 
+	if (rseq_cs->start_ip >= TASK_SIZE ||
+	    rseq_cs->start_ip + rseq_cs->post_commit_offset >= TASK_SIZE ||
+	    rseq_cs->abort_ip >= TASK_SIZE ||
+	    rseq_cs->version > 0)
+		return -EINVAL;
+	/* Check for overflow. */
+	if (rseq_cs->start_ip + rseq_cs->post_commit_offset < rseq_cs->start_ip)
+		return -EINVAL;
 	/* Ensure that abort_ip is not in the critical section. */
 	if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
 		return -EINVAL;
 
-	usig = (u32 __user *)(rseq_cs->abort_ip - sizeof(u32));
+	usig = (u32 __user *)(unsigned long)(rseq_cs->abort_ip - sizeof(u32));
 	ret = get_user(sig, usig);
 	if (ret)
 		return ret;
@@ -146,7 +152,7 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
 		printk_ratelimited(KERN_WARNING
 			"Possible attack attempt. Unexpected rseq signature 0x%x, expecting 0x%x (pid=%d, addr=%p).\n",
 			sig, current->rseq_sig, current->pid, usig);
-		return -EPERM;
+		return -EINVAL;
 	}
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related

* [RFC PATCH for 4.18 2/5] rseq: uapi: update uapi comments
From: Mathieu Desnoyers @ 2018-07-05 18:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20180705180601.18423-1-mathieu.desnoyers@efficios.com>

Update rseq uapi header comments to reflect that user-space need to do
thread-local loads/stores from/to the struct rseq fields.

As a consequence of this added requirement, the kernel does not need
to perform loads/stores with single-copy atomicity.

Update the comment associated to the "flags" fields to describe
more accurately that it's only useful to facilitate single-stepping
through rseq critical sections with debuggers.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 include/uapi/linux/rseq.h | 69 ++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index 519ad6e176d1..bf4188c13bec 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -67,28 +67,30 @@ struct rseq_cs {
 struct rseq {
 	/*
 	 * Restartable sequences cpu_id_start field. Updated by the
-	 * kernel, and read by user-space with single-copy atomicity
-	 * semantics. Aligned on 32-bit. Always contains a value in the
-	 * range of possible CPUs, although the value may not be the
-	 * actual current CPU (e.g. if rseq is not initialized). This
-	 * CPU number value should always be compared against the value
-	 * of the cpu_id field before performing a rseq commit or
-	 * returning a value read from a data structure indexed using
-	 * the cpu_id_start value.
+	 * kernel. Read by user-space with single-copy atomicity
+	 * semantics. This field should only be read by the thread which
+	 * registered this data structure. Aligned on 32-bit. Always
+	 * contains a value in the range of possible CPUs, although the
+	 * value may not be the actual current CPU (e.g. if rseq is not
+	 * initialized). This CPU number value should always be compared
+	 * against the value of the cpu_id field before performing a rseq
+	 * commit or returning a value read from a data structure indexed
+	 * using the cpu_id_start value.
 	 */
 	__u32 cpu_id_start;
 	/*
-	 * Restartable sequences cpu_id field. Updated by the kernel,
-	 * and read by user-space with single-copy atomicity semantics.
-	 * Aligned on 32-bit. Values RSEQ_CPU_ID_UNINITIALIZED and
-	 * RSEQ_CPU_ID_REGISTRATION_FAILED have a special semantic: the
-	 * former means "rseq uninitialized", and latter means "rseq
-	 * initialization failed". This value is meant to be read within
-	 * rseq critical sections and compared with the cpu_id_start
-	 * value previously read, before performing the commit instruction,
-	 * or read and compared with the cpu_id_start value before returning
-	 * a value loaded from a data structure indexed using the
-	 * cpu_id_start value.
+	 * Restartable sequences cpu_id field. Updated by the kernel.
+	 * Read by user-space with single-copy atomicity semantics. This
+	 * field should only be read by the thread which registered this
+	 * data structure. Aligned on 32-bit. Values
+	 * RSEQ_CPU_ID_UNINITIALIZED and RSEQ_CPU_ID_REGISTRATION_FAILED
+	 * have a special semantic: the former means "rseq uninitialized",
+	 * and latter means "rseq initialization failed". This value is
+	 * meant to be read within rseq critical sections and compared
+	 * with the cpu_id_start value previously read, before performing
+	 * the commit instruction, or read and compared with the
+	 * cpu_id_start value before returning a value loaded from a data
+	 * structure indexed using the cpu_id_start value.
 	 */
 	__u32 cpu_id;
 	/*
@@ -105,27 +107,28 @@ struct rseq {
 	 * targeted by the rseq_cs. Also needs to be set to NULL by user-space
 	 * before reclaiming memory that contains the targeted struct rseq_cs.
 	 *
-	 * Read and set by the kernel with single-copy atomicity semantics.
-	 * Set by user-space with single-copy atomicity semantics. Aligned
-	 * on 64-bit.
+	 * Read and set by the kernel. Set by user-space with single-copy
+	 * atomicity semantics. This field should only be updated by the
+	 * thread which registered this data structure. Aligned on 64-bit.
 	 */
 	LINUX_FIELD_u32_u64(rseq_cs);
 	/*
-	 * - RSEQ_DISABLE flag:
+	 * Restartable sequences flags field.
+	 *
+	 * This field should only be updated by the thread which
+	 * registered this data structure. Read by the kernel.
+	 * Mainly used for single-stepping through rseq critical sections
+	 * with debuggers.
 	 *
-	 * Fallback fast-track flag for single-stepping.
-	 * Set by user-space if lack of progress is detected.
-	 * Cleared by user-space after rseq finish.
-	 * Read by the kernel.
 	 * - RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT
-	 *     Inhibit instruction sequence block restart and event
-	 *     counter increment on preemption for this thread.
+	 *     Inhibit instruction sequence block restart on preemption
+	 *     for this thread.
 	 * - RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL
-	 *     Inhibit instruction sequence block restart and event
-	 *     counter increment on signal delivery for this thread.
+	 *     Inhibit instruction sequence block restart on signal
+	 *     delivery for this thread.
 	 * - RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE
-	 *     Inhibit instruction sequence block restart and event
-	 *     counter increment on migration for this thread.
+	 *     Inhibit instruction sequence block restart on migration for
+	 *     this thread.
 	 */
 	__u32 flags;
 } __attribute__((aligned(4 * sizeof(__u64))));
-- 
2.11.0

^ permalink raw reply related

* [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers @ 2018-07-05 18:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20180705180601.18423-1-mathieu.desnoyers@efficios.com>

Declaring the rseq_cs field as a union between __u64 and two __u32
allows both 32-bit and 64-bit kernels to read the full __u64, and
therefore validate that a 32-bit user-space cleared the upper 32
bits, thus ensuring a consistent behavior between native 32-bit
kernels and 32-bit compat tasks on 64-bit kernels.

Check that the rseq_cs value read is < TASK_SIZE.

The asm/byteorder.h header needs to be included by rseq.h, now
that it is not using linux/types_32_64.h anymore.

Considering that only __32 and __u64 types are declared in linux/rseq.h,
the linux/types.h header should always be included for both kernel and
user-space code: including stdint.h is just for u64 and u32, which are
not used in this header at all.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 include/uapi/linux/rseq.h           | 27 +++++++++++++++++++--------
 kernel/rseq.c                       | 12 +++++++-----
 tools/testing/selftests/rseq/rseq.h | 11 ++++++++++-
 3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index bf4188c13bec..9a402fdb60e9 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -10,13 +10,8 @@
  * Copyright (c) 2015-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  */
 
-#ifdef __KERNEL__
-# include <linux/types.h>
-#else
-# include <stdint.h>
-#endif
-
-#include <linux/types_32_64.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
 
 enum rseq_cpu_id_state {
 	RSEQ_CPU_ID_UNINITIALIZED		= -1,
@@ -111,7 +106,23 @@ struct rseq {
 	 * atomicity semantics. This field should only be updated by the
 	 * thread which registered this data structure. Aligned on 64-bit.
 	 */
-	LINUX_FIELD_u32_u64(rseq_cs);
+	union {
+		__u64 ptr64;
+#ifdef __LP64__
+		__u64 ptr;
+#else
+		struct {
+#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined(__BIG_ENDIAN)
+			__u32 padding;		/* Initialized to zero. */
+			__u32 ptr32;
+#else /* LITTLE */
+			__u32 ptr32;
+			__u32 padding;		/* Initialized to zero. */
+#endif /* ENDIAN */
+		} ptr;
+#endif
+	} rseq_cs;
+
 	/*
 	 * Restartable sequences flags field.
 	 *
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 16b38c5342f9..3081e6783cce 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -115,19 +115,21 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)
 static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
 {
 	struct rseq_cs __user *urseq_cs;
-	unsigned long ptr;
+	u64 ptr;
 	u32 __user *usig;
 	u32 sig;
 	int ret;
 
-	ret = __get_user(ptr, &t->rseq->rseq_cs);
+	ret = __get_user(ptr, &t->rseq->rseq_cs.ptr64);
 	if (ret)
 		return ret;
 	if (!ptr) {
 		memset(rseq_cs, 0, sizeof(*rseq_cs));
 		return 0;
 	}
-	urseq_cs = (struct rseq_cs __user *)ptr;
+	if (ptr >= TASK_SIZE)
+		return -EINVAL;
+	urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;
 	if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
 		return -EFAULT;
 
@@ -201,9 +203,9 @@ static int clear_rseq_cs(struct task_struct *t)
 	 * of code outside of the rseq assembly block. This performs
 	 * a lazy clear of the rseq_cs field.
 	 *
-	 * Set rseq_cs to NULL with single-copy atomicity.
+	 * Set rseq_cs to NULL.
 	 */
-	return __put_user(0UL, &t->rseq->rseq_cs);
+	return __put_user(0ULL, &t->rseq->rseq_cs.ptr64);
 }
 
 /*
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index a4684112676c..f2073cfa4448 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -133,6 +133,15 @@ static inline uint32_t rseq_current_cpu(void)
 	return cpu;
 }
 
+static inline void rseq_clear_rseq_cs(void)
+{
+#ifdef __LP64__
+	__rseq_abi.rseq_cs.ptr = 0;
+#else
+	__rseq_abi.rseq_cs.ptr.ptr32 = 0;
+#endif
+}
+
 /*
  * rseq_prepare_unload() should be invoked by each thread using rseq_finish*()
  * at least once between their last rseq_finish*() and library unload of the
@@ -143,7 +152,7 @@ static inline uint32_t rseq_current_cpu(void)
  */
 static inline void rseq_prepare_unload(void)
 {
-	__rseq_abi.rseq_cs = 0;
+	rseq_clear_rseq_cs();
 }
 
 #endif  /* RSEQ_H_ */
-- 
2.11.0

^ permalink raw reply related

* [RFC PATCH for 4.18 4/5] rseq: remove unused types_32_64.h uapi header
From: Mathieu Desnoyers @ 2018-07-05 18:06 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20180705180601.18423-1-mathieu.desnoyers@efficios.com>

This header was introduced in the 4.18 merge window, and rseq does
not need it anymore. Nuke it before the final release.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 include/uapi/linux/types_32_64.h | 50 ----------------------------------------
 1 file changed, 50 deletions(-)
 delete mode 100644 include/uapi/linux/types_32_64.h

diff --git a/include/uapi/linux/types_32_64.h b/include/uapi/linux/types_32_64.h
deleted file mode 100644
index 0a87ace34a57..000000000000
--- a/include/uapi/linux/types_32_64.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
-#ifndef _UAPI_LINUX_TYPES_32_64_H
-#define _UAPI_LINUX_TYPES_32_64_H
-
-/*
- * linux/types_32_64.h
- *
- * Integer type declaration for pointers across 32-bit and 64-bit systems.
- *
- * Copyright (c) 2015-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
- */
-
-#ifdef __KERNEL__
-# include <linux/types.h>
-#else
-# include <stdint.h>
-#endif
-
-#include <asm/byteorder.h>
-
-#ifdef __BYTE_ORDER
-# if (__BYTE_ORDER == __BIG_ENDIAN)
-#  define LINUX_BYTE_ORDER_BIG_ENDIAN
-# else
-#  define LINUX_BYTE_ORDER_LITTLE_ENDIAN
-# endif
-#else
-# ifdef __BIG_ENDIAN
-#  define LINUX_BYTE_ORDER_BIG_ENDIAN
-# else
-#  define LINUX_BYTE_ORDER_LITTLE_ENDIAN
-# endif
-#endif
-
-#ifdef __LP64__
-# define LINUX_FIELD_u32_u64(field)			__u64 field
-# define LINUX_FIELD_u32_u64_INIT_ONSTACK(field, v)	field = (intptr_t)v
-#else
-# ifdef LINUX_BYTE_ORDER_BIG_ENDIAN
-#  define LINUX_FIELD_u32_u64(field)	__u32 field ## _padding, field
-#  define LINUX_FIELD_u32_u64_INIT_ONSTACK(field, v)	\
-	field ## _padding = 0, field = (intptr_t)v
-# else
-#  define LINUX_FIELD_u32_u64(field)	__u32 field, field ## _padding
-#  define LINUX_FIELD_u32_u64_INIT_ONSTACK(field, v)	\
-	field = (intptr_t)v, field ## _padding = 0
-# endif
-#endif
-
-#endif /* _UAPI_LINUX_TYPES_32_64_H */
-- 
2.11.0

^ permalink raw reply related

* [RFC PATCH for 4.18 5/5] rseq/selftests: cleanup: update comment above rseq_prepare_unload
From: Mathieu Desnoyers @ 2018-07-05 18:06 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
	Michael Kerrisk
In-Reply-To: <20180705180601.18423-1-mathieu.desnoyers@efficios.com>

rseq as it was merged does not have rseq_finish_*() in the user-space
selftests anymore. Update the rseq_prepare_unload() helper comment to
adapt to this reality.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
 tools/testing/selftests/rseq/rseq.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index f2073cfa4448..86ce22417e0d 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -143,12 +143,13 @@ static inline void rseq_clear_rseq_cs(void)
 }
 
 /*
- * rseq_prepare_unload() should be invoked by each thread using rseq_finish*()
- * at least once between their last rseq_finish*() and library unload of the
- * library defining the rseq critical section (struct rseq_cs). This also
- * applies to use of rseq in code generated by JIT: rseq_prepare_unload()
- * should be invoked at least once by each thread using rseq_finish*() before
- * reclaim of the memory holding the struct rseq_cs.
+ * rseq_prepare_unload() should be invoked by each thread executing a rseq
+ * critical section at least once between their last critical section and
+ * library unload of the library defining the rseq critical section
+ * (struct rseq_cs). This also applies to use of rseq in code generated by
+ * JIT: rseq_prepare_unload() should be invoked at least once by each
+ * thread executing a rseq critical section before reclaim of the memory
+ * holding the struct rseq_cs.
  */
 static inline void rseq_prepare_unload(void)
 {
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2 0/7] Introduce struct __kernel_timex
From: Deepa Dinamani @ 2018-07-05 21:35 UTC (permalink / raw)
  To: tglx, linux-kernel
  Cc: arnd, y2038, catalin.marinas, davem, linux-alpha, linux-api,
	linux-arch, linux-riscv, netdev, palmer

The series introduces struct __kernel_timex as a substitute for
the non y2038 safe struct timex.

The series is based on the original series posted by Arnd Bergmann
in [1].

The overview of the series is as below:
1. Prepare for the compat timex interfaces to be used unconditionally.
2. Introduce struct __kernel_timex.
3. Use struct __kernel_timex in place of struct timex.
4. Switch syscalls to use struct __kernel_timex.

[1] https://sourceware.org/ml/libc-alpha/2015-05/msg00070.html

Changes since v1:
* Fix riscv asm/compat.h to pick up generic compat types

Deepa Dinamani (7):
  arm64: Make basic compat_* types always available
  sparc: Make thread_info.h available directly
  riscv: Include asm-generic/compat.h
  timex: prepare compat helpers for y2038 changes
  time: Add struct __kernel_timex
  timex: use __kernel_timex internally
  timex: change syscalls to use struct __kernel_timex

 arch/alpha/kernel/osf_sys.c     |  2 +-
 arch/arm64/include/asm/compat.h | 22 ++++-----
 arch/riscv/include/asm/compat.h |  3 ++
 arch/sparc/include/asm/compat.h |  2 +
 drivers/ptp/ptp_clock.c         |  2 +-
 include/asm-generic/compat.h    |  8 +++-
 include/linux/compat.h          | 33 --------------
 include/linux/compat_time.h     | 34 ++++++++++++++
 include/linux/posix-clock.h     |  2 +-
 include/linux/syscalls.h        |  5 +--
 include/linux/timex.h           |  9 +++-
 include/uapi/linux/timex.h      | 41 +++++++++++++++++
 kernel/compat.c                 | 63 --------------------------
 kernel/time/ntp.c               | 12 ++---
 kernel/time/ntp_internal.h      |  2 +-
 kernel/time/posix-clock.c       |  2 +-
 kernel/time/posix-timers.c      | 14 ++----
 kernel/time/posix-timers.h      |  2 +-
 kernel/time/time.c              | 80 ++++++++++++++++++++++++++++++---
 kernel/time/timekeeping.c       |  4 +-
 20 files changed, 201 insertions(+), 141 deletions(-)


base-commit: e30b8745c892204095c0a8b69405868f63ddcce1
-- 
2.17.1

Cc: catalin.marinas@arm.com
Cc: davem@davemloft.net
Cc: linux-alpha@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: netdev@vger.kernel.org
Cc: palmer@sifive.com

^ permalink raw reply

* [PATCH v2 7/7] timex: change syscalls to use struct __kernel_timex
From: Deepa Dinamani @ 2018-07-05 21:36 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: arnd, y2038, linux-api
In-Reply-To: <20180705213604.18883-1-deepa.kernel@gmail.com>

struct timex is not y2038 safe.
Switch all the syscall apis to use y2038 safe __kernel_timex.

Note that sys_adjtimex() does not have a y2038 safe solution.
The api is meant to be deprecated on 32 bit machines after y2038.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: linux-api@vger.kernel.org
---
 include/linux/syscalls.h   |  5 ++---
 kernel/time/posix-timers.c | 10 +---------
 kernel/time/time.c         |  9 +++++++--
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3ee3b3f1302f..54688c7b4dae 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -54,7 +54,6 @@ struct __sysctl_args;
 struct sysinfo;
 struct timespec;
 struct timeval;
-struct timex;
 struct timezone;
 struct tms;
 struct utimbuf;
@@ -677,7 +676,7 @@ asmlinkage long sys_gettimeofday(struct timeval __user *tv,
 				struct timezone __user *tz);
 asmlinkage long sys_settimeofday(struct timeval __user *tv,
 				struct timezone __user *tz);
-asmlinkage long sys_adjtimex(struct timex __user *txc_p);
+asmlinkage long sys_adjtimex(struct __kernel_timex __user *txc_p);
 
 /* kernel/timer.c */
 asmlinkage long sys_getpid(void);
@@ -846,7 +845,7 @@ asmlinkage long sys_open_by_handle_at(int mountdirfd,
 				      struct file_handle __user *handle,
 				      int flags);
 asmlinkage long sys_clock_adjtime(clockid_t which_clock,
-				struct timex __user *tx);
+				struct __kernel_timex __user *tx);
 asmlinkage long sys_syncfs(int fd);
 asmlinkage long sys_setns(int fd, int nstype);
 asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index a2595cb0cb16..1b485422f9f3 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1084,7 +1084,7 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,
 }
 
 SYSCALL_DEFINE2(clock_adjtime, const clockid_t, which_clock,
-		struct timex __user *, utx)
+		struct __kernel_timex __user *, utx)
 {
 	const struct k_clock *kc = clockid_to_kclock(which_clock);
 	struct __kernel_timex ktx;
@@ -1159,10 +1159,6 @@ COMPAT_SYSCALL_DEFINE2(clock_gettime, clockid_t, which_clock,
 	return err;
 }
 
-#endif
-
-#ifdef CONFIG_COMPAT
-
 COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,
 		       struct compat_timex __user *, utp)
 {
@@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,
 	return err;
 }
 
-#endif
-
-#ifdef CONFIG_COMPAT_32BIT_TIME
-
 COMPAT_SYSCALL_DEFINE2(clock_getres, clockid_t, which_clock,
 		       struct compat_timespec __user *, tp)
 {
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 2c5afb008b14..a374fdbb368b 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -263,7 +263,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct compat_timeval __user *, tv,
 }
 #endif
 
-SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)
+
+#if !defined(CONFIG_64BIT_TIME) || defined(CONFIG_64BIT)
+
+SYSCALL_DEFINE1(adjtimex, struct __kernel_timex __user *, txc_p)
 {
 	struct __kernel_timex txc;		/* Local copy of parameter */
 	int ret;
@@ -278,7 +281,9 @@ SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)
 	return copy_to_user(txc_p, &txc, sizeof(struct __kernel_timex)) ? -EFAULT : ret;
 }
 
-#ifdef CONFIG_COMPAT
+#endif
+
+#ifdef CONFIG_COMPAT_32BIT_TIME
 
 COMPAT_SYSCALL_DEFINE1(adjtimex, struct compat_timex __user *, utp)
 {
-- 
2.17.1

^ permalink raw reply related

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers @ 2018-07-06 16:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E. McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H. Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
	Linus Torvalds, Catalin Marinas, Will Deacon, Michael
In-Reply-To: <20180705180601.18423-4-mathieu.desnoyers@efficios.com>

----- On Jul 5, 2018, at 2:05 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> Declaring the rseq_cs field as a union between __u64 and two __u32
> allows both 32-bit and 64-bit kernels to read the full __u64, and
> therefore validate that a 32-bit user-space cleared the upper 32
> bits, thus ensuring a consistent behavior between native 32-bit
> kernels and 32-bit compat tasks on 64-bit kernels.
> 
> Check that the rseq_cs value read is < TASK_SIZE.
> 
> The asm/byteorder.h header needs to be included by rseq.h, now
> that it is not using linux/types_32_64.h anymore.
> 
> Considering that only __32 and __u64 types are declared in linux/rseq.h,
> the linux/types.h header should always be included for both kernel and
> user-space code: including stdint.h is just for u64 and u32, which are
> not used in this header at all.

The 0-day bot noticed that __get_user() is unimplemented for 64-bit
values on arm32 (although get_user() is implemented).

The following diff fixes this discrepancy, and allows this rseq patch
to build on arm32:

commit dde99f3310c76acb0a160c0572f40b6aa279594c
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date:   Fri Jul 6 11:29:39 2018 -0400

    arm: implement 64-bit __get_user
    
    get_user() is implemented on arm32 for 64-bit user-space values, but
    not its __get_user() counterpart.
    
    Implement __get_user() as two __get_user_asm_word().
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
    CC: Peter Zijlstra <peterz@infradead.org>
    CC: Paul Turner <pjt@google.com>
    CC: Thomas Gleixner <tglx@linutronix.de>
    CC: Andy Lutomirski <luto@amacapital.net>
    CC: Andi Kleen <andi@firstfloor.org>
    CC: Dave Watson <davejwatson@fb.com>
    CC: Chris Lameter <cl@linux.com>
    CC: Ingo Molnar <mingo@redhat.com>
    CC: "H. Peter Anvin" <hpa@zytor.com>
    CC: Ben Maurer <bmaurer@fb.com>
    CC: Steven Rostedt <rostedt@goodmis.org>
    CC: Josh Triplett <josh@joshtriplett.org>
    CC: Linus Torvalds <torvalds@linux-foundation.org>
    CC: Andrew Morton <akpm@linux-foundation.org>
    CC: Russell King <linux@arm.linux.org.uk>
    CC: Catalin Marinas <catalin.marinas@arm.com>
    CC: Will Deacon <will.deacon@arm.com>
    CC: Michael Kerrisk <mtk.manpages@gmail.com>
    CC: Boqun Feng <boqun.feng@gmail.com>
    CC: linux-api@vger.kernel.org

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 3d614e9..38659c6 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -265,10 +265,16 @@ static inline void set_fs(mm_segment_t fs)
        (void) 0;                                                       \
 })
 
+union __gu_u64 {
+       u64 val64;
+       u32 word[2];
+};
+
 #define __get_user_err(x, ptr, err)                                    \
 do {                                                                   \
        unsigned long __gu_addr = (unsigned long)(ptr);                 \
        unsigned long __gu_val;                                         \
+       union __gu_u64 __gu_tmp64;                                      \
        unsigned int __ua_flags;                                        \
        __chk_user_ptr(ptr);                                            \
        might_fault();                                                  \
@@ -277,10 +283,28 @@ static inline void set_fs(mm_segment_t fs)
        case 1: __get_user_asm_byte(__gu_val, __gu_addr, err);  break;  \
        case 2: __get_user_asm_half(__gu_val, __gu_addr, err);  break;  \
        case 4: __get_user_asm_word(__gu_val, __gu_addr, err);  break;  \
+       case 8:                                                         \
+       {                                                               \
+               union __gu_u64 __user *__gu_addr64 =                    \
+                       (union __gu_u64 __user *)__gu_addr;             \
+               __get_user_asm_word(__gu_tmp64.word[0],                 \
+                        &__gu_addr64->word[0], err);                   \
+               if (err)                                                \
+                       break;                                          \
+               __get_user_asm_word(__gu_tmp64.word[1],                 \
+                        &__gu_addr64->word[1], err);                   \
+               break;                                                  \
+       };                                                              \
        default: (__gu_val) = __get_user_bad();                         \
        }                                                               \
        uaccess_restore(__ua_flags);                                    \
-       (x) = (__typeof__(*(ptr)))__gu_val;                             \
+       switch (sizeof(*(ptr))) {                                       \
+       case 1:                                                         \
+       case 2:                                                         \
+       case 4:                                                         \
+       default: (x) = (__typeof__(*(ptr)))__gu_val; break;             \
+       case 8: (x) = (__typeof__(*(ptr)))__gu_tmp64.val64; break;      \
+       }                                                               \
 } while (0)
 
 #define __get_user_asm(x, addr, err, instr)                    \


> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Paul Turner <pjt@google.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Andi Kleen <andi@firstfloor.org>
> CC: Dave Watson <davejwatson@fb.com>
> CC: Chris Lameter <cl@linux.com>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Ben Maurer <bmaurer@fb.com>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Josh Triplett <josh@joshtriplett.org>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: linux-api@vger.kernel.org
> ---
> include/uapi/linux/rseq.h           | 27 +++++++++++++++++++--------
> kernel/rseq.c                       | 12 +++++++-----
> tools/testing/selftests/rseq/rseq.h | 11 ++++++++++-
> 3 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
> index bf4188c13bec..9a402fdb60e9 100644
> --- a/include/uapi/linux/rseq.h
> +++ b/include/uapi/linux/rseq.h
> @@ -10,13 +10,8 @@
>  * Copyright (c) 2015-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>  */
> 
> -#ifdef __KERNEL__
> -# include <linux/types.h>
> -#else
> -# include <stdint.h>
> -#endif
> -
> -#include <linux/types_32_64.h>
> +#include <linux/types.h>
> +#include <asm/byteorder.h>
> 
> enum rseq_cpu_id_state {
> 	RSEQ_CPU_ID_UNINITIALIZED		= -1,
> @@ -111,7 +106,23 @@ struct rseq {
> 	 * atomicity semantics. This field should only be updated by the
> 	 * thread which registered this data structure. Aligned on 64-bit.
> 	 */
> -	LINUX_FIELD_u32_u64(rseq_cs);
> +	union {
> +		__u64 ptr64;
> +#ifdef __LP64__
> +		__u64 ptr;
> +#else
> +		struct {
> +#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) ||
> defined(__BIG_ENDIAN)
> +			__u32 padding;		/* Initialized to zero. */
> +			__u32 ptr32;
> +#else /* LITTLE */
> +			__u32 ptr32;
> +			__u32 padding;		/* Initialized to zero. */
> +#endif /* ENDIAN */
> +		} ptr;
> +#endif
> +	} rseq_cs;
> +
> 	/*
> 	 * Restartable sequences flags field.
> 	 *
> diff --git a/kernel/rseq.c b/kernel/rseq.c
> index 16b38c5342f9..3081e6783cce 100644
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -115,19 +115,21 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)
> static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
> {
> 	struct rseq_cs __user *urseq_cs;
> -	unsigned long ptr;
> +	u64 ptr;
> 	u32 __user *usig;
> 	u32 sig;
> 	int ret;
> 
> -	ret = __get_user(ptr, &t->rseq->rseq_cs);
> +	ret = __get_user(ptr, &t->rseq->rseq_cs.ptr64);
> 	if (ret)
> 		return ret;
> 	if (!ptr) {
> 		memset(rseq_cs, 0, sizeof(*rseq_cs));
> 		return 0;
> 	}
> -	urseq_cs = (struct rseq_cs __user *)ptr;
> +	if (ptr >= TASK_SIZE)
> +		return -EINVAL;
> +	urseq_cs = (struct rseq_cs __user *)(unsigned long)ptr;
> 	if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
> 		return -EFAULT;
> 
> @@ -201,9 +203,9 @@ static int clear_rseq_cs(struct task_struct *t)
> 	 * of code outside of the rseq assembly block. This performs
> 	 * a lazy clear of the rseq_cs field.
> 	 *
> -	 * Set rseq_cs to NULL with single-copy atomicity.
> +	 * Set rseq_cs to NULL.
> 	 */
> -	return __put_user(0UL, &t->rseq->rseq_cs);
> +	return __put_user(0ULL, &t->rseq->rseq_cs.ptr64);
> }
> 
> /*
> diff --git a/tools/testing/selftests/rseq/rseq.h
> b/tools/testing/selftests/rseq/rseq.h
> index a4684112676c..f2073cfa4448 100644
> --- a/tools/testing/selftests/rseq/rseq.h
> +++ b/tools/testing/selftests/rseq/rseq.h
> @@ -133,6 +133,15 @@ static inline uint32_t rseq_current_cpu(void)
> 	return cpu;
> }
> 
> +static inline void rseq_clear_rseq_cs(void)
> +{
> +#ifdef __LP64__
> +	__rseq_abi.rseq_cs.ptr = 0;
> +#else
> +	__rseq_abi.rseq_cs.ptr.ptr32 = 0;
> +#endif
> +}
> +
> /*
>  * rseq_prepare_unload() should be invoked by each thread using rseq_finish*()
>  * at least once between their last rseq_finish*() and library unload of the
> @@ -143,7 +152,7 @@ static inline uint32_t rseq_current_cpu(void)
>  */
> static inline void rseq_prepare_unload(void)
> {
> -	__rseq_abi.rseq_cs = 0;
> +	rseq_clear_rseq_cs();
> }
> 
> #endif  /* RSEQ_H_ */
> --
> 2.11.0

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply related

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers @ 2018-07-06 19:23 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E. McKenney,
	Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
	Andrew Morton, Russell King, Ingo Molnar, H. Peter Anvin,
	Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
	Linus Torvalds, Catalin Marinas, Will Deacon, Michael
In-Reply-To: <1846432971.1245.1530892973439.JavaMail.zimbra@efficios.com>

----- On Jul 6, 2018, at 12:02 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Jul 5, 2018, at 2:05 PM, Mathieu Desnoyers
> mathieu.desnoyers@efficios.com wrote:
> 
[...]
> The 0-day bot noticed that __get_user() is unimplemented for 64-bit
> values on arm32 (although get_user() is implemented).
> 
> The following diff fixes this discrepancy, and allows this rseq patch
> to build on arm32:
> 

For -rc, I would favor the following simpler approach. Or I could even
just use get_user() instead. Thoughts ?

    rseq: implement work-around for missing 8-byte __get_user on arm
    
    Now that rseq uses __u64 for its pointer fields, 32-bit architectures
    need to read this 64-bit value from user-space.
    
    __get_user is used to read this value, given that its access check has
    already been performed with access_ok() on rseq registration.
    
    arm does not implement 8-byte __get_user. Work-around this limitation
    by using get_user() on ARM instead, with its redundant access check.
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    CC: Thomas Gleixner <tglx@linutronix.de>
    Cc: Joel Fernandes <joelaf@google.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Catalin Marinas <catalin.marinas@arm.com>
    Cc: Dave Watson <davejwatson@fb.com>
    Cc: Will Deacon <will.deacon@arm.com>
    Cc: Andi Kleen <andi@firstfloor.org>
    Cc: "H . Peter Anvin" <hpa@zytor.com>
    Cc: Chris Lameter <cl@linux.com>
    Cc: Russell King <linux@arm.linux.org.uk>
    Cc: Andrew Hunter <ahh@google.com>
    Cc: Michael Kerrisk <mtk.manpages@gmail.com>
    Cc: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
    Cc: Paul Turner <pjt@google.com>
    Cc: Boqun Feng <boqun.feng@gmail.com>
    Cc: Josh Triplett <josh@joshtriplett.org>
    Cc: Steven Rostedt <rostedt@goodmis.org>
    Cc: Ben Maurer <bmaurer@fb.com>
    Cc: linux-api@vger.kernel.org
    CC: linux-arm-kernel@lists.infradead.org
    Cc: Andy Lutomirski <luto@amacapital.net>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/kernel/rseq.c b/kernel/rseq.c
index 3081e67..0e67625 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -18,6 +18,16 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/rseq.h>
 
+/*
+ * ARM does not implement 8 bytes __get_user. Use get_user on that
+ * architecture instead.
+ */
+#ifdef CONFIG_ARM
+#define __rseq_get_user                get_user
+#else
+#define __rseq_get_user                __get_user
+#endif
+
 #define RSEQ_CS_PREEMPT_MIGRATE_FLAGS (RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE | \
                                       RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)
 
@@ -120,7 +130,7 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rs
        u32 sig;
        int ret;
 
-       ret = __get_user(ptr, &t->rseq->rseq_cs.ptr64);
+       ret = __rseq_get_user(ptr, &t->rseq->rseq_cs.ptr64);
        if (ret)
                return ret;
        if (!ptr) {



-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply related

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Linus Torvalds @ 2018-07-06 19:31 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, Linux Kernel Mailing List, Linux API,
	Peter Zijlstra, Paul McKenney, Boqun Feng, Andy Lutomirski,
	Dave Watson, Paul Turner, Andrew Morton, Russell King - ARM Linux,
	Ingo Molnar, Peter Anvin, Andi Kleen, Christoph Lameter,
	Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas,
	Will Deacon
In-Reply-To: <436937568.1359.1530905019620.JavaMail.zimbra@efficios.com>

On Fri, Jul 6, 2018 at 12:23 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> For -rc, I would favor the following simpler approach. Or I could even
> just use get_user() instead. Thoughts ?

Please just use "get_user()".

In fact, we should be thinking seriosly about just removing
__get_user() entirely. It's wrong. It optimizes the wrong thing
entirely. It _used_ to be that the range check was noticeable, and it
really isn't any more. These days the expensive parts are the SMAP
costs, and both get_user() and __get_user() have those, except
get_user() is safer and doesn't waste I$ on inlining the code to
disable and re-enable SMAP.

                Linus

^ permalink raw reply

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers @ 2018-07-06 19:35 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thomas Gleixner, linux-kernel, linux-api, Peter Zijlstra,
	Paul E. McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
	Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
	H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will Deacon,
	Michael Kerrisk <mtk.ma>
In-Reply-To: <CA+55aFzz1CC6son0T2nD-Yqhy0_yC5nBer2XSH9kqe5x3N26Ug@mail.gmail.com>

----- On Jul 6, 2018, at 3:31 PM, Linus Torvalds torvalds@linux-foundation.org wrote:

> On Fri, Jul 6, 2018 at 12:23 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> For -rc, I would favor the following simpler approach. Or I could even
>> just use get_user() instead. Thoughts ?
> 
> Please just use "get_user()".
> 
> In fact, we should be thinking seriosly about just removing
> __get_user() entirely. It's wrong. It optimizes the wrong thing
> entirely. It _used_ to be that the range check was noticeable, and it
> really isn't any more. These days the expensive parts are the SMAP
> costs, and both get_user() and __get_user() have those, except
> get_user() is safer and doesn't waste I$ on inlining the code to
> disable and re-enable SMAP.

Will do, thanks!

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Mathieu Desnoyers @ 2018-07-06 19:38 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thomas Gleixner, linux-kernel, linux-api, Peter Zijlstra,
	Paul E. McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
	Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
	H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Catalin Marinas, Will Deacon,
	Michael Kerrisk <mtk.ma>
In-Reply-To: <1913049637.1366.1530905741745.JavaMail.zimbra@efficios.com>

----- On Jul 6, 2018, at 3:35 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Jul 6, 2018, at 3:31 PM, Linus Torvalds torvalds@linux-foundation.org
> wrote:
> 
>> On Fri, Jul 6, 2018 at 12:23 PM Mathieu Desnoyers
>> <mathieu.desnoyers@efficios.com> wrote:
>>>
>>> For -rc, I would favor the following simpler approach. Or I could even
>>> just use get_user() instead. Thoughts ?
>> 
>> Please just use "get_user()".
>> 
>> In fact, we should be thinking seriosly about just removing
>> __get_user() entirely. It's wrong. It optimizes the wrong thing
>> entirely. It _used_ to be that the range check was noticeable, and it
>> really isn't any more. These days the expensive parts are the SMAP
>> costs, and both get_user() and __get_user() have those, except
>> get_user() is safer and doesn't waste I$ on inlining the code to
>> disable and re-enable SMAP.
> 
> Will do, thanks!

Should I change all 4 bytes __get_user()/__put_user() in kernel/rseq.c
for get_user()/put_user() to ensure consistency ?

Thanks,

Mathieu

> 
> Mathieu
> 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Andy Lutomirski @ 2018-07-06 19:56 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Mathieu Desnoyers, Thomas Gleixner, Linux Kernel Mailing List,
	Linux API, Peter Zijlstra, Paul McKenney, Boqun Feng, Dave Watson,
	Paul Turner, Andrew Morton, Russell King - ARM Linux, Ingo Molnar,
	Peter Anvin, Andi Kleen, Christoph Lameter, Ben Maurer,
	Steven Rostedt, Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <CA+55aFzz1CC6son0T2nD-Yqhy0_yC5nBer2XSH9kqe5x3N26Ug@mail.gmail.com>

On Fri, Jul 6, 2018 at 12:31 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jul 6, 2018 at 12:23 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> For -rc, I would favor the following simpler approach. Or I could even
>> just use get_user() instead. Thoughts ?
>
> Please just use "get_user()".
>
> In fact, we should be thinking seriosly about just removing
> __get_user() entirely. It's wrong. It optimizes the wrong thing
> entirely. It _used_ to be that the range check was noticeable, and it
> really isn't any more. These days the expensive parts are the SMAP
> costs, and both get_user() and __get_user() have those, except
> get_user() is safer and doesn't waste I$ on inlining the code to
> disable and re-enable SMAP.

If Al and Christoph ever manage to get rid of set_fs(), I bet we can
rewrite access_ok() and get_user() so that gcc can fold redundant
checks together and generate optimal code for get_user() of
consecutive struct fields all by itself.  Or maybe I'm giving gcc more
credit than it deserves.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 3/5] rseq: uapi: declare rseq_cs field as union, update includes
From: Linus Torvalds @ 2018-07-06 19:56 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, Linux Kernel Mailing List, Linux API,
	Peter Zijlstra, Paul McKenney, Boqun Feng, Andy Lutomirski,
	Dave Watson, Paul Turner, Andrew Morton, Russell King - ARM Linux,
	Ingo Molnar, Peter Anvin, Andi Kleen, Christoph Lameter,
	Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas,
	Will Deacon
In-Reply-To: <131696830.1369.1530905931822.JavaMail.zimbra@efficios.com>

On Fri, Jul 6, 2018 at 12:38 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> Should I change all 4 bytes __get_user()/__put_user() in kernel/rseq.c
> for get_user()/put_user() to ensure consistency ?

Probably.

*If* this actually turns out to be somethinig that shows up on
profiles, it's almost certainly going to be the STAC/CLAC instructions
("perf report" tends to report them as three one-byte nop's because
that's how they look before instruction replacement).

And then it's not __get/put_user() that will improve things, but doing a

        user_access_begin();

        .. do unsafe_get/put_user() ..

        user_access_end();

that will improve performance.

But it is *very* seldom useful. We have it in a handful of places in
the kernel, and the most noticeable one is
lib/{strnlen,strncpy_from}_user.c

                       Linus

^ permalink raw reply

* [PATCH v3 0/7] Introduce struct __kernel_timex
From: Deepa Dinamani @ 2018-07-07  5:42 UTC (permalink / raw)
  To: tglx, linux-kernel
  Cc: linux-arch, palmer, arnd, y2038, catalin.marinas, linux-alpha,
	netdev, linux-api, linux-riscv, davem

The series introduces struct __kernel_timex as a substitute for
the non y2038 safe struct timex.

The series is based on the original series posted by Arnd Bergmann
in [1].

The overview of the series is as below:
1. Prepare for the compat timex interfaces to be used unconditionally.
2. Introduce struct __kernel_timex.
3. Use struct __kernel_timex in place of struct timex.
4. Switch syscalls to use struct __kernel_timex.

[1] https://sourceware.org/ml/libc-alpha/2015-05/msg00070.html

Changes since v2:
* Use only generic compat.h for riscv
Changes since v1:
* Fix riscv asm/compat.h to pick up generic compat types

Deepa Dinamani (7):
  arm64: Make basic compat_* types always available
  sparc: Make thread_info.h available directly
  riscv: Delete asm/compat.h
  timex: prepare compat helpers for y2038 changes
  time: Add struct __kernel_timex
  timex: use __kernel_timex internally
  timex: change syscalls to use struct __kernel_timex

 arch/alpha/kernel/osf_sys.c     |  2 +-
 arch/arm64/include/asm/compat.h | 22 ++++-----
 arch/riscv/include/asm/Kbuild   |  1 +
 arch/riscv/include/asm/compat.h | 29 ------------
 arch/sparc/include/asm/compat.h |  2 +
 drivers/ptp/ptp_clock.c         |  2 +-
 include/asm-generic/compat.h    |  8 +++-
 include/linux/compat.h          | 33 --------------
 include/linux/compat_time.h     | 34 ++++++++++++++
 include/linux/posix-clock.h     |  2 +-
 include/linux/syscalls.h        |  5 +--
 include/linux/timex.h           |  9 +++-
 include/uapi/linux/timex.h      | 41 +++++++++++++++++
 kernel/compat.c                 | 63 --------------------------
 kernel/time/ntp.c               | 12 ++---
 kernel/time/ntp_internal.h      |  2 +-
 kernel/time/posix-clock.c       |  2 +-
 kernel/time/posix-timers.c      | 14 ++----
 kernel/time/posix-timers.h      |  2 +-
 kernel/time/time.c              | 80 ++++++++++++++++++++++++++++++---
 kernel/time/timekeeping.c       |  4 +-
 21 files changed, 199 insertions(+), 170 deletions(-)
 delete mode 100644 arch/riscv/include/asm/compat.h


base-commit: 526674536360a4c508e84f67314c2028e45e1bf2
-- 
2.17.1

Cc: catalin.marinas@arm.com
Cc: davem@davemloft.net
Cc: linux-alpha@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: netdev@vger.kernel.org
Cc: palmer@sifive.com
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply

* [PATCH v3 7/7] timex: change syscalls to use struct __kernel_timex
From: Deepa Dinamani @ 2018-07-07  5:42 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: arnd, y2038, linux-api
In-Reply-To: <20180707054247.19802-1-deepa.kernel@gmail.com>

struct timex is not y2038 safe.
Switch all the syscall apis to use y2038 safe __kernel_timex.

Note that sys_adjtimex() does not have a y2038 safe solution.
The api is meant to be deprecated on 32 bit machines after y2038.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: linux-api@vger.kernel.org
---
 include/linux/syscalls.h   |  5 ++---
 kernel/time/posix-timers.c | 10 +---------
 kernel/time/time.c         |  9 +++++++--
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3ee3b3f1302f..54688c7b4dae 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -54,7 +54,6 @@ struct __sysctl_args;
 struct sysinfo;
 struct timespec;
 struct timeval;
-struct timex;
 struct timezone;
 struct tms;
 struct utimbuf;
@@ -677,7 +676,7 @@ asmlinkage long sys_gettimeofday(struct timeval __user *tv,
 				struct timezone __user *tz);
 asmlinkage long sys_settimeofday(struct timeval __user *tv,
 				struct timezone __user *tz);
-asmlinkage long sys_adjtimex(struct timex __user *txc_p);
+asmlinkage long sys_adjtimex(struct __kernel_timex __user *txc_p);
 
 /* kernel/timer.c */
 asmlinkage long sys_getpid(void);
@@ -846,7 +845,7 @@ asmlinkage long sys_open_by_handle_at(int mountdirfd,
 				      struct file_handle __user *handle,
 				      int flags);
 asmlinkage long sys_clock_adjtime(clockid_t which_clock,
-				struct timex __user *tx);
+				struct __kernel_timex __user *tx);
 asmlinkage long sys_syncfs(int fd);
 asmlinkage long sys_setns(int fd, int nstype);
 asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index a2595cb0cb16..1b485422f9f3 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1084,7 +1084,7 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,
 }
 
 SYSCALL_DEFINE2(clock_adjtime, const clockid_t, which_clock,
-		struct timex __user *, utx)
+		struct __kernel_timex __user *, utx)
 {
 	const struct k_clock *kc = clockid_to_kclock(which_clock);
 	struct __kernel_timex ktx;
@@ -1159,10 +1159,6 @@ COMPAT_SYSCALL_DEFINE2(clock_gettime, clockid_t, which_clock,
 	return err;
 }
 
-#endif
-
-#ifdef CONFIG_COMPAT
-
 COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,
 		       struct compat_timex __user *, utp)
 {
@@ -1187,10 +1183,6 @@ COMPAT_SYSCALL_DEFINE2(clock_adjtime, clockid_t, which_clock,
 	return err;
 }
 
-#endif
-
-#ifdef CONFIG_COMPAT_32BIT_TIME
-
 COMPAT_SYSCALL_DEFINE2(clock_getres, clockid_t, which_clock,
 		       struct compat_timespec __user *, tp)
 {
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 2c5afb008b14..a374fdbb368b 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -263,7 +263,10 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct compat_timeval __user *, tv,
 }
 #endif
 
-SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)
+
+#if !defined(CONFIG_64BIT_TIME) || defined(CONFIG_64BIT)
+
+SYSCALL_DEFINE1(adjtimex, struct __kernel_timex __user *, txc_p)
 {
 	struct __kernel_timex txc;		/* Local copy of parameter */
 	int ret;
@@ -278,7 +281,9 @@ SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)
 	return copy_to_user(txc_p, &txc, sizeof(struct __kernel_timex)) ? -EFAULT : ret;
 }
 
-#ifdef CONFIG_COMPAT
+#endif
+
+#ifdef CONFIG_COMPAT_32BIT_TIME
 
 COMPAT_SYSCALL_DEFINE1(adjtimex, struct compat_timex __user *, utp)
 {
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] mtdchar: fix overflows in adjustment of `count`
From: Jann Horn @ 2018-07-07  9:03 UTC (permalink / raw)
  To: boris.brezillon
  Cc: richard, kernel list, marek.vasut, linux-mtd, Linux API,
	computersforpeace, dwmw2
In-Reply-To: <20180707104412.1580a285@bbrezillon>

+cc linux-api

On Sat, Jul 7, 2018 at 10:44 AM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
>
> On Sat,  7 Jul 2018 05:37:22 +0200
> Jann Horn <jannh@google.com> wrote:
>
> > The first checks in mtdchar_read() and mtdchar_write() attempt to limit
> > `count` such that `*ppos + count <= mtd->size`. However, they ignore the
> > possibility of `*ppos > mtd->size`, allowing the calculation of `count` to
> > wrap around. `mtdchar_lseek()` prevents seeking beyond mtd->size, but the
> > pread/pwrite syscalls bypass this.
> >
> > I haven't found any codepath on which this actually causes dangerous
> > behavior, but it seems like a sensible change anyway.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> >  drivers/mtd/mtdchar.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> > index cd67c85cc87d..02389528f622 100644
> > --- a/drivers/mtd/mtdchar.c
> > +++ b/drivers/mtd/mtdchar.c
> > @@ -160,8 +160,12 @@ static ssize_t mtdchar_read(struct file *file, char __user *buf, size_t count,
> >
> >       pr_debug("MTD_read\n");
> >
> > -     if (*ppos + count > mtd->size)
> > -             count = mtd->size - *ppos;
> > +     if (*ppos + count > mtd->size) {
> > +             if (*ppos < mtd->size)
> > +                     count = mtd->size - *ppos;
> > +             else
> > +                     count = 0;
> > +     }
>
> Hm, shouldn't we return -ERANGE or -EINVAL if *ppos >= mtd->size?

Hmm, good question.
The pread() manpage says that pread() can return the same errors as
lseek(), and the lseek() manpage says that -EINVAL is for when you're
trying to seek beyond the end of a seekable device. So from the
documentation, it sounds as if you're right.
But testing pread() beyond end of file for various things on my
machine seems to just return 0:

# cat pread.c
#include <unistd.h>
#include <stdlib.h>
int main(int argc, char **argv) {
  char buf[0x1000];
  off_t off = strtoul(argv[1], NULL, 0);
  pread(0, buf, 0x1000, off);
}
# gcc -o pread pread.c
# strace -e trace=pread64 ./pread 200000000 < /dev/sda1
pread64(0, "", 4096, 200000000)         = 0
+++ exited with 0 +++
# strace -e trace=pread64 ./pread 100000 <
/sys/kernel/debug/x86/tlb_single_page_flush_ceiling
pread64(0, "", 4096, 100000)            = 0
+++ exited with 0 +++
# strace -e trace=pread64 ./pread 20000000000 < /dev/dm-2
pread64(0, "", 4096, 20000000000)       = 0
+++ exited with 0 +++

Do you know of precedent for returning -EINVAL if *ppos is beyond the
end of the device?

> >
> >       if (!count)
> >               return 0;
> > @@ -246,7 +250,7 @@ static ssize_t mtdchar_write(struct file *file, const char __user *buf, size_t c
> >
> >       pr_debug("MTD_write\n");
> >
> > -     if (*ppos == mtd->size)
> > +     if (*ppos >= mtd->size)
> >               return -ENOSPC;
> >
> >       if (*ppos + count > mtd->size)
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply


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