From: Mark Rutland <mark.rutland@arm.com>
To: Andy Lutomirski <luto@amacapital.net>
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: Wed, 21 Sep 2016 11:28:27 +0100 [thread overview]
Message-ID: <20160921102827.GC18176@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
Hi Andy,
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:
> > 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().
While fixing up these patches, I realised that I'm somewhat concerned by
flags becoming a u32 (where it was previously an unsigned long for
arm64).
The generic {test,set,*}_ti_thread_flag() helpers use the usual bitops,
which perform accesses of sizeof(unsigned long) at a time, and for arm64
these need to be naturally-aligned.
We happen to get that alignment from subsequent fields in task_struct
and/or thread_info, and for arm64 we don't seem to have a problem with
tearing, but it feels somewhat fragile, and leaves me uneasy.
Looking at the git log, it seems that x86 also use unsigned long until
commit affa219b60a11b32 ("x86: change thread_info's flag field back to
32 bits"), where if I'm reading correctly, this was done to get rid of
unnecessary padding. With THREAD_INFO_IN_STACK, thread_info::flags is
immediately followed by a long on x86, so we save no padding.
Given all that, can we make the generic thread_info::flags an unsigned
long, matching what the thread flag helpers implicitly assume?
Thanks,
Mark.
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: Wed, 21 Sep 2016 11:28:27 +0100 [thread overview]
Message-ID: <20160921102827.GC18176@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
Hi Andy,
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:
> > 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().
While fixing up these patches, I realised that I'm somewhat concerned by
flags becoming a u32 (where it was previously an unsigned long for
arm64).
The generic {test,set,*}_ti_thread_flag() helpers use the usual bitops,
which perform accesses of sizeof(unsigned long) at a time, and for arm64
these need to be naturally-aligned.
We happen to get that alignment from subsequent fields in task_struct
and/or thread_info, and for arm64 we don't seem to have a problem with
tearing, but it feels somewhat fragile, and leaves me uneasy.
Looking at the git log, it seems that x86 also use unsigned long until
commit affa219b60a11b32 ("x86: change thread_info's flag field back to
32 bits"), where if I'm reading correctly, this was done to get rid of
unnecessary padding. With THREAD_INFO_IN_STACK, thread_info::flags is
immediately followed by a long on x86, so we save no padding.
Given all that, can we make the generic thread_info::flags an unsigned
long, matching what the thread flag helpers implicitly assume?
Thanks,
Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Andy Lutomirski <luto@amacapital.net>
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: Wed, 21 Sep 2016 11:28:27 +0100 [thread overview]
Message-ID: <20160921102827.GC18176@leverpostej> (raw)
In-Reply-To: <CALCETrUDWiDmLbMqw9sQYEzvDuSVfmT_UscxQXfmf8YYyTOC=Q@mail.gmail.com>
Hi Andy,
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:
> > 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().
While fixing up these patches, I realised that I'm somewhat concerned by
flags becoming a u32 (where it was previously an unsigned long for
arm64).
The generic {test,set,*}_ti_thread_flag() helpers use the usual bitops,
which perform accesses of sizeof(unsigned long) at a time, and for arm64
these need to be naturally-aligned.
We happen to get that alignment from subsequent fields in task_struct
and/or thread_info, and for arm64 we don't seem to have a problem with
tearing, but it feels somewhat fragile, and leaves me uneasy.
Looking at the git log, it seems that x86 also use unsigned long until
commit affa219b60a11b32 ("x86: change thread_info's flag field back to
32 bits"), where if I'm reading correctly, this was done to get rid of
unnecessary padding. With THREAD_INFO_IN_STACK, thread_info::flags is
immediately followed by a long on x86, so we save no padding.
Given all that, can we make the generic thread_info::flags an unsigned
long, matching what the thread flag helpers implicitly assume?
Thanks,
Mark.
next prev parent reply other threads:[~2016-09-21 10:28 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 ` [kernel-hardening] " Mark Rutland
2016-09-19 10:44 ` Mark Rutland
2016-09-19 10:44 ` Mark Rutland
2016-09-21 10:28 ` Mark Rutland [this message]
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=20160921102827.GC18176@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 \
/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.