| Message ID | 1537970184-44348-2-git-send-email-julien.thierry@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Ensure stack is aligned for kernel entries | expand |
On Wed, Sep 26, 2018 at 02:56:18PM +0100, Julien Thierry wrote: > The pt_regs structure's size is required to be a multiple of 16 to maintain > the stack alignment upon kernel entries. > > Add a static check to ensure this is the case. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > --- > arch/arm64/include/asm/ptrace.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index 177b851..cc17fad 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) > > WARN_ON(offset & 7); > > + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); This seems like a fairly random place to put this, and given that it's not dependent on context I reckon it would be better to put it in a C file rather than in a header that gets included multiple times. I still couldn't really find a good place though :/ Will
On Wed, Nov 07, 2018 at 09:58:35PM +0000, Will Deacon wrote: > On Wed, Sep 26, 2018 at 02:56:18PM +0100, Julien Thierry wrote: > > The pt_regs structure's size is required to be a multiple of 16 to maintain > > the stack alignment upon kernel entries. > > > > Add a static check to ensure this is the case. > > > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > > --- > > arch/arm64/include/asm/ptrace.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > > index 177b851..cc17fad 100644 > > --- a/arch/arm64/include/asm/ptrace.h > > +++ b/arch/arm64/include/asm/ptrace.h > > @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) > > > > WARN_ON(offset & 7); > > > > + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); > > This seems like a fairly random place to put this, and given that it's not > dependent on context I reckon it would be better to put it in a C file > rather than in a header that gets included multiple times. I still couldn't > really find a good place though :/ Perhaps asm-offsets.c, when we define S_FRAME_SIZE? Thanks, Mark.
On 08/11/18 10:16, Mark Rutland wrote: > On Wed, Nov 07, 2018 at 09:58:35PM +0000, Will Deacon wrote: >> On Wed, Sep 26, 2018 at 02:56:18PM +0100, Julien Thierry wrote: >>> The pt_regs structure's size is required to be a multiple of 16 to maintain >>> the stack alignment upon kernel entries. >>> >>> Add a static check to ensure this is the case. >>> >>> Signed-off-by: Julien Thierry <julien.thierry@arm.com> >>> --- >>> arch/arm64/include/asm/ptrace.h | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h >>> index 177b851..cc17fad 100644 >>> --- a/arch/arm64/include/asm/ptrace.h >>> +++ b/arch/arm64/include/asm/ptrace.h >>> @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) >>> >>> WARN_ON(offset & 7); >>> >>> + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); >> >> This seems like a fairly random place to put this, and given that it's not >> dependent on context I reckon it would be better to put it in a C file >> rather than in a header that gets included multiple times. I still couldn't >> really find a good place though :/ > > Perhaps asm-offsets.c, when we define S_FRAME_SIZE? > That sounds like a good option to me. If Will thinks so as well I'll move that in asm-offsets.c. Thanks,
On Thu, Nov 08, 2018 at 11:57:27AM +0000, Julien Thierry wrote: > > > On 08/11/18 10:16, Mark Rutland wrote: > > On Wed, Nov 07, 2018 at 09:58:35PM +0000, Will Deacon wrote: > > > On Wed, Sep 26, 2018 at 02:56:18PM +0100, Julien Thierry wrote: > > > > The pt_regs structure's size is required to be a multiple of 16 to maintain > > > > the stack alignment upon kernel entries. > > > > > > > > Add a static check to ensure this is the case. > > > > > > > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > > > > --- > > > > arch/arm64/include/asm/ptrace.h | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > > > > index 177b851..cc17fad 100644 > > > > --- a/arch/arm64/include/asm/ptrace.h > > > > +++ b/arch/arm64/include/asm/ptrace.h > > > > @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) > > > > WARN_ON(offset & 7); > > > > + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); > > > > > > This seems like a fairly random place to put this, and given that it's not > > > dependent on context I reckon it would be better to put it in a C file > > > rather than in a header that gets included multiple times. I still couldn't > > > really find a good place though :/ > > > > Perhaps asm-offsets.c, when we define S_FRAME_SIZE? > > > > That sounds like a good option to me. If Will thinks so as well I'll move > that in asm-offsets.c. Fine by me. Will
On 26 September 2018 at 15:56, Julien Thierry <julien.thierry@arm.com> wrote: > The pt_regs structure's size is required to be a multiple of 16 to maintain > the stack alignment upon kernel entries. > > Add a static check to ensure this is the case. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > --- > arch/arm64/include/asm/ptrace.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index 177b851..cc17fad 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) > > WARN_ON(offset & 7); > > + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); The parens inside BUILD_BUG_ON() are redundant (unless you move the closing one right after '16', which I would prefer) > + > offset >>= 3; > switch (offset) { > case 0 ... 30: > -- > 1.9.1 > Wherever you manage to put this: Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index 177b851..cc17fad 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -229,6 +229,8 @@ static inline u64 regs_get_register(struct pt_regs *regs, unsigned int offset) WARN_ON(offset & 7); + BUILD_BUG_ON((sizeof (struct pt_regs) % 16 != 0)); + offset >>= 3; switch (offset) { case 0 ... 30:
The pt_regs structure's size is required to be a multiple of 16 to maintain the stack alignment upon kernel entries. Add a static check to ensure this is the case. Signed-off-by: Julien Thierry <julien.thierry@arm.com> --- arch/arm64/include/asm/ptrace.h | 2 ++ 1 file changed, 2 insertions(+)