From: Mark Rutland <mark.rutland@arm.com>
To: Andy Lutomirski <luto@amacapital.net>,
Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Catalin Marinas <catalin.marinas@arm.com>,
james.morse@arm.com, Kees Cook <keescook@chromium.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
lorenzo.pieralisi@arm.com, Andrew Lutomirski <luto@kernel.org>,
suzuki.poulose@arm.com,
Takahiro Akashi <takahiro.akashi@linaro.org>,
Will Deacon <will.deacon@arm.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>
Subject: [kernel-hardening] Re: [RFC PATCH 2/8] thread_info: allow custom in-task thread_info
Date: Mon, 19 Sep 2016 11:44:51 +0100 [thread overview]
Message-ID: <20160919104254.GA12473@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
[Adding Yosinori Sato for h8300]
On Fri, Sep 16, 2016 at 08:11:14AM -0700, Andy Lutomirski wrote:
> > On Thu, Sep 15, 2016 at 11:37:47AM -0700, Andy Lutomirski wrote:
> > > On Thu, Sep 15, 2016 at 6:49 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Just to check, what do you mean to happen with the flags field? Should
> > that always be in the generic thread_info? e.g.
> >
> > struct thread_info {
> > u32 flags;
> > #ifdef arch_thread_info
> > struct arch_thread_info arch_ti;
> > #endif
> > };
>
> Exactly. Possibly with a comment that using thread_struct should be
> preferred and that arch_thread_info should be used only if some header
> file requires access via current_thread_info() or task_thread_info().
Sure thing.
While looking at that, I spotted that would cause a circular include on
h8300, but that appears to be because of a larger bug anyhow. It seems
h8300 has thread_info::restart_block, and manipulates this in its signal
code, yet core code manipulates task_struct::restart_block.
I think we need something like the below (which is completely untested).
Thanks,
Mark.
---->8----
>From 4dfbdc7706bfb03a48999ff69e85e0c7361adffe Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 19 Sep 2016 11:33:42 +0100
Subject: [PATCH] h8300: fix syscall restarting
Back in commit f56141e3e2d9aabf ("all arches, signal: move restart_block
to struct task_struct"), all architectures and core code were changed to
use task_struct::restart_block. However, when h8300 support was
subsequently restored in v4.2, it was not updated to account for this,
and maintains thread_info::restart_block, which is not kept in sync.
This patch drops the redundant restart_block from thread_info, and moves
h8300 to the common one in task_struct, ensuring that syscall restarting
always works as expected.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: stable@vger.kernel.org # v4.2+
---
arch/h8300/include/asm/thread_info.h | 4 ----
arch/h8300/kernel/signal.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/h8300/include/asm/thread_info.h b/arch/h8300/include/asm/thread_info.h
index b408fe6..3cef068 100644
--- a/arch/h8300/include/asm/thread_info.h
+++ b/arch/h8300/include/asm/thread_info.h
@@ -31,7 +31,6 @@ struct thread_info {
int cpu; /* cpu we're on */
int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit;
- struct restart_block restart_block;
};
/*
@@ -44,9 +43,6 @@ struct thread_info {
.cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
.addr_limit = KERNEL_DS, \
- .restart_block = { \
- .fn = do_no_restart_syscall, \
- }, \
}
#define init_thread_info (init_thread_union.thread_info)
diff --git a/arch/h8300/kernel/signal.c b/arch/h8300/kernel/signal.c
index ad1f81f..7138303 100644
--- a/arch/h8300/kernel/signal.c
+++ b/arch/h8300/kernel/signal.c
@@ -79,7 +79,7 @@ restore_sigcontext(struct sigcontext *usc, int *pd0)
unsigned int er0;
/* Always make any pending restarted system calls return -EINTR */
- current_thread_info()->restart_block.fn = do_no_restart_syscall;
+ current->restart_block.fn = do_no_restart_syscall;
/* restore passed registers */
#define COPY(r) do { err |= get_user(regs->r, &usc->sc_##r); } while (0)
--
1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 2/8] thread_info: allow custom in-task thread_info
Date: Mon, 19 Sep 2016 11:44:51 +0100 [thread overview]
Message-ID: <20160919104254.GA12473@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
[Adding Yosinori Sato for h8300]
On Fri, Sep 16, 2016 at 08:11:14AM -0700, Andy Lutomirski wrote:
> > On Thu, Sep 15, 2016 at 11:37:47AM -0700, Andy Lutomirski wrote:
> > > On Thu, Sep 15, 2016 at 6:49 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Just to check, what do you mean to happen with the flags field? Should
> > that always be in the generic thread_info? e.g.
> >
> > struct thread_info {
> > u32 flags;
> > #ifdef arch_thread_info
> > struct arch_thread_info arch_ti;
> > #endif
> > };
>
> Exactly. Possibly with a comment that using thread_struct should be
> preferred and that arch_thread_info should be used only if some header
> file requires access via current_thread_info() or task_thread_info().
Sure thing.
While looking at that, I spotted that would cause a circular include on
h8300, but that appears to be because of a larger bug anyhow. It seems
h8300 has thread_info::restart_block, and manipulates this in its signal
code, yet core code manipulates task_struct::restart_block.
I think we need something like the below (which is completely untested).
Thanks,
Mark.
---->8----
>From 4dfbdc7706bfb03a48999ff69e85e0c7361adffe Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 19 Sep 2016 11:33:42 +0100
Subject: [PATCH] h8300: fix syscall restarting
Back in commit f56141e3e2d9aabf ("all arches, signal: move restart_block
to struct task_struct"), all architectures and core code were changed to
use task_struct::restart_block. However, when h8300 support was
subsequently restored in v4.2, it was not updated to account for this,
and maintains thread_info::restart_block, which is not kept in sync.
This patch drops the redundant restart_block from thread_info, and moves
h8300 to the common one in task_struct, ensuring that syscall restarting
always works as expected.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: stable at vger.kernel.org # v4.2+
---
arch/h8300/include/asm/thread_info.h | 4 ----
arch/h8300/kernel/signal.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/h8300/include/asm/thread_info.h b/arch/h8300/include/asm/thread_info.h
index b408fe6..3cef068 100644
--- a/arch/h8300/include/asm/thread_info.h
+++ b/arch/h8300/include/asm/thread_info.h
@@ -31,7 +31,6 @@ struct thread_info {
int cpu; /* cpu we're on */
int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit;
- struct restart_block restart_block;
};
/*
@@ -44,9 +43,6 @@ struct thread_info {
.cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
.addr_limit = KERNEL_DS, \
- .restart_block = { \
- .fn = do_no_restart_syscall, \
- }, \
}
#define init_thread_info (init_thread_union.thread_info)
diff --git a/arch/h8300/kernel/signal.c b/arch/h8300/kernel/signal.c
index ad1f81f..7138303 100644
--- a/arch/h8300/kernel/signal.c
+++ b/arch/h8300/kernel/signal.c
@@ -79,7 +79,7 @@ restore_sigcontext(struct sigcontext *usc, int *pd0)
unsigned int er0;
/* Always make any pending restarted system calls return -EINTR */
- current_thread_info()->restart_block.fn = do_no_restart_syscall;
+ current->restart_block.fn = do_no_restart_syscall;
/* restore passed registers */
#define COPY(r) do { err |= get_user(regs->r, &usc->sc_##r); } while (0)
--
1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Andy Lutomirski <luto@amacapital.net>,
Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Catalin Marinas <catalin.marinas@arm.com>,
james.morse@arm.com, Kees Cook <keescook@chromium.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
lorenzo.pieralisi@arm.com, Andrew Lutomirski <luto@kernel.org>,
suzuki.poulose@arm.com,
Takahiro Akashi <takahiro.akashi@linaro.org>,
Will Deacon <will.deacon@arm.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>
Subject: Re: [RFC PATCH 2/8] thread_info: allow custom in-task thread_info
Date: Mon, 19 Sep 2016 11:44:51 +0100 [thread overview]
Message-ID: <20160919104254.GA12473@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
[Adding Yosinori Sato for h8300]
On Fri, Sep 16, 2016 at 08:11:14AM -0700, Andy Lutomirski wrote:
> > On Thu, Sep 15, 2016 at 11:37:47AM -0700, Andy Lutomirski wrote:
> > > On Thu, Sep 15, 2016 at 6:49 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Just to check, what do you mean to happen with the flags field? Should
> > that always be in the generic thread_info? e.g.
> >
> > struct thread_info {
> > u32 flags;
> > #ifdef arch_thread_info
> > struct arch_thread_info arch_ti;
> > #endif
> > };
>
> Exactly. Possibly with a comment that using thread_struct should be
> preferred and that arch_thread_info should be used only if some header
> file requires access via current_thread_info() or task_thread_info().
Sure thing.
While looking at that, I spotted that would cause a circular include on
h8300, but that appears to be because of a larger bug anyhow. It seems
h8300 has thread_info::restart_block, and manipulates this in its signal
code, yet core code manipulates task_struct::restart_block.
I think we need something like the below (which is completely untested).
Thanks,
Mark.
---->8----
>From 4dfbdc7706bfb03a48999ff69e85e0c7361adffe Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 19 Sep 2016 11:33:42 +0100
Subject: [PATCH] h8300: fix syscall restarting
Back in commit f56141e3e2d9aabf ("all arches, signal: move restart_block
to struct task_struct"), all architectures and core code were changed to
use task_struct::restart_block. However, when h8300 support was
subsequently restored in v4.2, it was not updated to account for this,
and maintains thread_info::restart_block, which is not kept in sync.
This patch drops the redundant restart_block from thread_info, and moves
h8300 to the common one in task_struct, ensuring that syscall restarting
always works as expected.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: stable@vger.kernel.org # v4.2+
---
arch/h8300/include/asm/thread_info.h | 4 ----
arch/h8300/kernel/signal.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/h8300/include/asm/thread_info.h b/arch/h8300/include/asm/thread_info.h
index b408fe6..3cef068 100644
--- a/arch/h8300/include/asm/thread_info.h
+++ b/arch/h8300/include/asm/thread_info.h
@@ -31,7 +31,6 @@ struct thread_info {
int cpu; /* cpu we're on */
int preempt_count; /* 0 => preemptable, <0 => BUG */
mm_segment_t addr_limit;
- struct restart_block restart_block;
};
/*
@@ -44,9 +43,6 @@ struct thread_info {
.cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
.addr_limit = KERNEL_DS, \
- .restart_block = { \
- .fn = do_no_restart_syscall, \
- }, \
}
#define init_thread_info (init_thread_union.thread_info)
diff --git a/arch/h8300/kernel/signal.c b/arch/h8300/kernel/signal.c
index ad1f81f..7138303 100644
--- a/arch/h8300/kernel/signal.c
+++ b/arch/h8300/kernel/signal.c
@@ -79,7 +79,7 @@ restore_sigcontext(struct sigcontext *usc, int *pd0)
unsigned int er0;
/* Always make any pending restarted system calls return -EINTR */
- current_thread_info()->restart_block.fn = do_no_restart_syscall;
+ current->restart_block.fn = do_no_restart_syscall;
/* restore passed registers */
#define COPY(r) do { err |= get_user(regs->r, &usc->sc_##r); } while (0)
--
1.9.1
next prev parent reply other threads:[~2016-09-19 10:44 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-15 13:49 [kernel-hardening] [RFC PATCH 0/8] arm64: move thread_info off of the task stack Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 1/8] thread_info: include <current.h> for THREAD_INFO_IN_TASK Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 2/8] thread_info: allow custom in-task thread_info Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 18:37 ` [kernel-hardening] " Andy Lutomirski
2016-09-15 18:37 ` Andy Lutomirski
2016-09-15 18:37 ` Andy Lutomirski
2016-09-16 10:33 ` [kernel-hardening] " Mark Rutland
2016-09-16 10:33 ` Mark Rutland
2016-09-16 10:33 ` Mark Rutland
2016-09-16 15:11 ` [kernel-hardening] " Andy Lutomirski
2016-09-16 15:11 ` Andy Lutomirski
2016-09-16 15:11 ` Andy Lutomirski
2016-09-19 10:44 ` Mark Rutland [this message]
2016-09-19 10:44 ` Mark Rutland
2016-09-19 10:44 ` Mark Rutland
2016-09-21 10:28 ` [kernel-hardening] " Mark Rutland
2016-09-21 10:28 ` Mark Rutland
2016-09-21 10:28 ` Mark Rutland
2016-09-22 22:23 ` [kernel-hardening] " Andy Lutomirski
2016-09-22 22:23 ` Andy Lutomirski
2016-09-22 22:23 ` Andy Lutomirski
2016-09-23 17:31 ` [kernel-hardening] " Mark Rutland
2016-09-23 17:31 ` Mark Rutland
2016-09-23 17:31 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 3/8] arm64: thread_info remove stale items Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 4/8] arm64: asm-offsets: remove unused definitions Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 5/8] arm64: assembler: introduce ldr_this_cpu Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 6/8] arm64: traps: use task_struct instead of thread_info Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 7/8] arm64: move sp_el0 and tpidr_el1 into cpu_suspend_ctx Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` [kernel-hardening] [RFC PATCH 8/8] arm64: split thread_info from task stack Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-15 13:49 ` Mark Rutland
2016-09-21 1:26 ` [kernel-hardening] Re: [RFC PATCH 0/8] arm64: move thread_info off of the " Laura Abbott
2016-09-21 1:26 ` Laura Abbott
2016-09-21 1:26 ` Laura Abbott
2016-09-21 10:31 ` [kernel-hardening] " Mark Rutland
2016-09-21 10:31 ` Mark Rutland
2016-09-21 10:31 ` Mark Rutland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160919104254.GA12473@leverpostej \
--to=mark.rutland@arm.com \
--cc=akpm@linux-foundation.org \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=takahiro.akashi@linaro.org \
--cc=will.deacon@arm.com \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.