LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
@ 2015-03-09 14:52 Denys Vlasenko
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Denys Vlasenko @ 2015-03-09 14:52 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

Suggested by Linus.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 include/linux/stddef.h |  9 +++++++++
 include/linux/vfio.h   | 13 -------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index f4aec0e..076af43 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -19,3 +19,12 @@ enum {
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 #endif
 #endif
+
+/**
+ * offsetofend(TYPE, MEMBER)
+ *
+ * @TYPE: The type of the structure
+ * @MEMBER: The member within the structure to get the end offset of
+ */
+#define offsetofend(TYPE, MEMBER) \
+	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 2d67b89..049b2f4 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -78,19 +78,6 @@ extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
 extern void vfio_unregister_iommu_driver(
 				const struct vfio_iommu_driver_ops *ops);
 
-/**
- * offsetofend(TYPE, MEMBER)
- *
- * @TYPE: The type of the structure
- * @MEMBER: The member within the structure to get the end offset of
- *
- * Simple helper macro for dealing with variable sized structures passed
- * from user space.  This allows us to easily determine if the provided
- * structure is sized to include various fields.
- */
-#define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
-
 /*
  * External user API
  */
-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented
  2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
@ 2015-03-09 14:52 ` Denys Vlasenko
  2015-03-09 15:16   ` Andy Lutomirski
                     ` (3 more replies)
  2015-03-09 14:58 ` [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Ingo Molnar
                   ` (3 subsequent siblings)
  4 siblings, 4 replies; 16+ messages in thread
From: Denys Vlasenko @ 2015-03-09 14:52 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

Before the patch, tss.stack field was not referenced anywhere.
It was used only by setting sysenter's stack to point after
last byte of tss, thus the trailing field, stack[64], was used.

But grep would not know it. You can comment it out, compile,
and kernel will even run until an unlucky NMI corrupts
io_bitmap[] (which is also not easily detectable).

This patch changes code so that the purpose and usage of this field
is not mysterious anymore, and can be easily grepped for.

This does change generated code, for a subtle reason:
since tss_struct is ____cacheline_aligned, there happen to be
5 longs of padding at the end. Old code was using the padding too;
new code will strictly use only SYSENTER_stack[].

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
Changes since v1: use offsetofend()

 arch/x86/include/asm/processor.h | 4 ++--
 arch/x86/kernel/asm-offsets_32.c | 2 +-
 arch/x86/kernel/cpu/common.c     | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 48a61c1..9e65cf8 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -276,9 +276,9 @@ struct tss_struct {
 	unsigned long		io_bitmap[IO_BITMAP_LONGS + 1];
 
 	/*
-	 * .. and then another 0x100 bytes for the emergency kernel stack:
+	 * and then space for temporary SYSENTER stack:
 	 */
-	unsigned long		stack[64];
+	unsigned long		SYSENTER_stack[64];
 
 } ____cacheline_aligned;
 
diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
index 3b3b9d3..42a3b28 100644
--- a/arch/x86/kernel/asm-offsets_32.c
+++ b/arch/x86/kernel/asm-offsets_32.c
@@ -68,7 +68,7 @@ void foo(void)
 
 	/* Offset from the sysenter stack to tss.sp0 */
 	DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) -
-		 sizeof(struct tss_struct));
+		offsetofend(struct tss_struct, SYSENTER_stack));
 
 #if defined(CONFIG_LGUEST) || defined(CONFIG_LGUEST_GUEST) || defined(CONFIG_LGUEST_MODULE)
 	BLANK();
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7634833..4701293 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -987,7 +987,8 @@ void enable_sep_cpu(void)
 	}
 
 	tss->x86_tss.ss1 = __KERNEL_CS;
-	tss->x86_tss.sp1 = sizeof(struct tss_struct) + (unsigned long) tss;
+	tss->x86_tss.sp1 = (unsigned long) tss
+			+ offsetofend(struct tss_struct, SYSENTER_stack);
 	wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 	wrmsr(MSR_IA32_SYSENTER_ESP, tss->x86_tss.sp1, 0);
 	wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long) ia32_sysenter_target, 0);
-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
@ 2015-03-09 14:58 ` Ingo Molnar
  2015-03-09 15:15   ` Denys Vlasenko
  2015-03-09 16:16 ` Linus Torvalds
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2015-03-09 14:58 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Andy Lutomirski, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> Suggested by Linus.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  include/linux/stddef.h |  9 +++++++++
>  include/linux/vfio.h   | 13 -------------
>  2 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index f4aec0e..076af43 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -19,3 +19,12 @@ enum {
>  #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
>  #endif
>  #endif
> +
> +/**
> + * offsetofend(TYPE, MEMBER)
> + *
> + * @TYPE: The type of the structure
> + * @MEMBER: The member within the structure to get the end offset of
> + */
> +#define offsetofend(TYPE, MEMBER) \
> +	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 2d67b89..049b2f4 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -78,19 +78,6 @@ extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
>  extern void vfio_unregister_iommu_driver(
>  				const struct vfio_iommu_driver_ops *ops);
>  
> -/**
> - * offsetofend(TYPE, MEMBER)
> - *
> - * @TYPE: The type of the structure
> - * @MEMBER: The member within the structure to get the end offset of
> - *
> - * Simple helper macro for dealing with variable sized structures passed
> - * from user space.  This allows us to easily determine if the provided
> - * structure is sized to include various fields.
> - */
> -#define offsetofend(TYPE, MEMBER) \
> -	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))

So I like it, and because it is not particularly trivial when to use 
this primitive it was explained nicely in a description in the vfio.h 
version.

But you lost that nice description during the code move!!

Please don't.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 14:58 ` [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Ingo Molnar
@ 2015-03-09 15:15   ` Denys Vlasenko
  2015-03-09 15:28     ` Ingo Molnar
  0 siblings, 1 reply; 16+ messages in thread
From: Denys Vlasenko @ 2015-03-09 15:15 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Andy Lutomirski, Linus Torvalds, Steven Rostedt,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	X86 ML, Linux Kernel Mailing List

On Mon, Mar 9, 2015 at 3:58 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> -/**
>> - * offsetofend(TYPE, MEMBER)
>> - *
>> - * @TYPE: The type of the structure
>> - * @MEMBER: The member within the structure to get the end offset of
>> - *
>> - * Simple helper macro for dealing with variable sized structures passed
>> - * from user space.  This allows us to easily determine if the provided
>> - * structure is sized to include various fields.
>> - */
>> -#define offsetofend(TYPE, MEMBER) \
>> -     (offsetof(TYPE, MEMBER) + sizeof(((TYPE *)0)->MEMBER))
>
> So I like it, and because it is not particularly trivial when to use
> this primitive it was explained nicely in a description in the vfio.h
> version.
>
> But you lost that nice description during the code move!!

That description was clearly specific to how that macro is used in
drivers/vfio/*.c, along the lines of

                minsz = offsetofend(struct vfio_eeh_pe_op, op);
                if (copy_from_user(&op, (void __user *)arg, minsz))
                        return -EFAULT;
                if (op.argsz < minsz || op.flags)
                        return -EINVAL;

But the macro is generic, it has many other uses besides this one.

Nevertheless, I can resend a version where comment survives if you want...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
@ 2015-03-09 15:16   ` Andy Lutomirski
  2015-03-14 16:00   ` Pavel Machek
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Andy Lutomirski @ 2015-03-09 15:16 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Linus Torvalds, Steven Rostedt, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, X86 ML, linux-kernel

On Mon, Mar 9, 2015 at 7:52 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> Before the patch, tss.stack field was not referenced anywhere.
> It was used only by setting sysenter's stack to point after
> last byte of tss, thus the trailing field, stack[64], was used.
>
> But grep would not know it. You can comment it out, compile,
> and kernel will even run until an unlucky NMI corrupts
> io_bitmap[] (which is also not easily detectable).
>
> This patch changes code so that the purpose and usage of this field
> is not mysterious anymore, and can be easily grepped for.
>
> This does change generated code, for a subtle reason:
> since tss_struct is ____cacheline_aligned, there happen to be
> 5 longs of padding at the end. Old code was using the padding too;
> new code will strictly use only SYSENTER_stack[].

Acked-by: Andy Lutomirski <luto@amacapital.net>

>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> Changes since v1: use offsetofend()
>
>  arch/x86/include/asm/processor.h | 4 ++--
>  arch/x86/kernel/asm-offsets_32.c | 2 +-
>  arch/x86/kernel/cpu/common.c     | 3 ++-
>  3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 48a61c1..9e65cf8 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -276,9 +276,9 @@ struct tss_struct {
>         unsigned long           io_bitmap[IO_BITMAP_LONGS + 1];
>
>         /*
> -        * .. and then another 0x100 bytes for the emergency kernel stack:
> +        * and then space for temporary SYSENTER stack:
>          */
> -       unsigned long           stack[64];
> +       unsigned long           SYSENTER_stack[64];
>
>  } ____cacheline_aligned;
>
> diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
> index 3b3b9d3..42a3b28 100644
> --- a/arch/x86/kernel/asm-offsets_32.c
> +++ b/arch/x86/kernel/asm-offsets_32.c
> @@ -68,7 +68,7 @@ void foo(void)
>
>         /* Offset from the sysenter stack to tss.sp0 */
>         DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) -
> -                sizeof(struct tss_struct));
> +               offsetofend(struct tss_struct, SYSENTER_stack));
>
>  #if defined(CONFIG_LGUEST) || defined(CONFIG_LGUEST_GUEST) || defined(CONFIG_LGUEST_MODULE)
>         BLANK();
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 7634833..4701293 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -987,7 +987,8 @@ void enable_sep_cpu(void)
>         }
>
>         tss->x86_tss.ss1 = __KERNEL_CS;
> -       tss->x86_tss.sp1 = sizeof(struct tss_struct) + (unsigned long) tss;
> +       tss->x86_tss.sp1 = (unsigned long) tss
> +                       + offsetofend(struct tss_struct, SYSENTER_stack);
>         wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
>         wrmsr(MSR_IA32_SYSENTER_ESP, tss->x86_tss.sp1, 0);
>         wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long) ia32_sysenter_target, 0);
> --
> 1.8.1.4
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 15:15   ` Denys Vlasenko
@ 2015-03-09 15:28     ` Ingo Molnar
  2015-03-09 15:30       ` Andy Lutomirski
  2015-03-09 15:44       ` Alex Williamson
  0 siblings, 2 replies; 16+ messages in thread
From: Ingo Molnar @ 2015-03-09 15:28 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Denys Vlasenko, Andy Lutomirski, Linus Torvalds, Steven Rostedt,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	X86 ML, Linux Kernel Mailing List


* Denys Vlasenko <vda.linux@googlemail.com> wrote:

> On Mon, Mar 9, 2015 at 3:58 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >> -/**
> >> - * offsetofend(TYPE, MEMBER)
> >> - *
> >> - * @TYPE: The type of the structure
> >> - * @MEMBER: The member within the structure to get the end offset of
> >> - *
> >> - * Simple helper macro for dealing with variable sized structures passed
> >> - * from user space.  This allows us to easily determine if the provided
> >> - * structure is sized to include various fields.
> >> - */
> >> -#define offsetofend(TYPE, MEMBER) \
> >> -     (offsetof(TYPE, MEMBER) + sizeof(((TYPE *)0)->MEMBER))
> >
> > So I like it, and because it is not particularly trivial when to use
> > this primitive it was explained nicely in a description in the vfio.h
> > version.
> >
> > But you lost that nice description during the code move!!
> 
> That description was clearly specific to how that macro is used in
> drivers/vfio/*.c, along the lines of
> 
>                 minsz = offsetofend(struct vfio_eeh_pe_op, op);

Hm, but here 'minsz' == sizeof(struct vfio_eeh_pe_op), so the vfio 
usage does not seem to be justified.

>                 if (copy_from_user(&op, (void __user *)arg, minsz))
>                         return -EFAULT;
>                 if (op.argsz < minsz || op.flags)
>                         return -EINVAL;
> 
> But the macro is generic, it has many other uses besides this one.

So I might be missing something, but what generic uses does it have, 
beyond structures that have some rare size related weirdness, such as 
alignment attributes? In 99% of the cases:

   sizeof(struct) == offsetofend(struct, last_member)

right?

> Nevertheless, I can resend a version where comment survives if you 
> want...

So maybe extend it to a description that you think describes its uses 
correctly? People will keep wondering about when to use this.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 15:28     ` Ingo Molnar
@ 2015-03-09 15:30       ` Andy Lutomirski
  2015-03-09 15:45         ` Ingo Molnar
  2015-03-09 15:44       ` Alex Williamson
  1 sibling, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2015-03-09 15:30 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Denys Vlasenko, Linus Torvalds, Steven Rostedt,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	X86 ML, Linux Kernel Mailing List

On Mon, Mar 9, 2015 at 8:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Denys Vlasenko <vda.linux@googlemail.com> wrote:
>
>> On Mon, Mar 9, 2015 at 3:58 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >> -/**
>> >> - * offsetofend(TYPE, MEMBER)
>> >> - *
>> >> - * @TYPE: The type of the structure
>> >> - * @MEMBER: The member within the structure to get the end offset of
>> >> - *
>> >> - * Simple helper macro for dealing with variable sized structures passed
>> >> - * from user space.  This allows us to easily determine if the provided
>> >> - * structure is sized to include various fields.
>> >> - */
>> >> -#define offsetofend(TYPE, MEMBER) \
>> >> -     (offsetof(TYPE, MEMBER) + sizeof(((TYPE *)0)->MEMBER))
>> >
>> > So I like it, and because it is not particularly trivial when to use
>> > this primitive it was explained nicely in a description in the vfio.h
>> > version.
>> >
>> > But you lost that nice description during the code move!!
>>
>> That description was clearly specific to how that macro is used in
>> drivers/vfio/*.c, along the lines of
>>
>>                 minsz = offsetofend(struct vfio_eeh_pe_op, op);
>
> Hm, but here 'minsz' == sizeof(struct vfio_eeh_pe_op), so the vfio
> usage does not seem to be justified.
>
>>                 if (copy_from_user(&op, (void __user *)arg, minsz))
>>                         return -EFAULT;
>>                 if (op.argsz < minsz || op.flags)
>>                         return -EINVAL;
>>
>> But the macro is generic, it has many other uses besides this one.
>
> So I might be missing something, but what generic uses does it have,
> beyond structures that have some rare size related weirdness, such as
> alignment attributes? In 99% of the cases:
>
>    sizeof(struct) == offsetofend(struct, last_member)
>
> right?

struct foo {
    u64 a;
    char b;
};

sizeof(struct foo) will be 16, but offsetofend(struct foo, b) will be
9 on most platforms, right?

--Andy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 15:28     ` Ingo Molnar
  2015-03-09 15:30       ` Andy Lutomirski
@ 2015-03-09 15:44       ` Alex Williamson
  1 sibling, 0 replies; 16+ messages in thread
From: Alex Williamson @ 2015-03-09 15:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Denys Vlasenko, Andy Lutomirski, Linus Torvalds,
	Steven Rostedt, Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	X86 ML, Linux Kernel Mailing List

On Mon, 2015-03-09 at 16:28 +0100, Ingo Molnar wrote:
> * Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> > On Mon, Mar 9, 2015 at 3:58 PM, Ingo Molnar <mingo@kernel.org> wrote:
> > >> -/**
> > >> - * offsetofend(TYPE, MEMBER)
> > >> - *
> > >> - * @TYPE: The type of the structure
> > >> - * @MEMBER: The member within the structure to get the end offset of
> > >> - *
> > >> - * Simple helper macro for dealing with variable sized structures passed
> > >> - * from user space.  This allows us to easily determine if the provided
> > >> - * structure is sized to include various fields.
> > >> - */
> > >> -#define offsetofend(TYPE, MEMBER) \
> > >> -     (offsetof(TYPE, MEMBER) + sizeof(((TYPE *)0)->MEMBER))
> > >
> > > So I like it, and because it is not particularly trivial when to use
> > > this primitive it was explained nicely in a description in the vfio.h
> > > version.
> > >
> > > But you lost that nice description during the code move!!
> > 
> > That description was clearly specific to how that macro is used in
> > drivers/vfio/*.c, along the lines of
> > 
> >                 minsz = offsetofend(struct vfio_eeh_pe_op, op);
> 
> Hm, but here 'minsz' == sizeof(struct vfio_eeh_pe_op), so the vfio 
> usage does not seem to be justified.
> 
> >                 if (copy_from_user(&op, (void __user *)arg, minsz))
> >                         return -EFAULT;
> >                 if (op.argsz < minsz || op.flags)
> >                         return -EINVAL;
> > 
> > But the macro is generic, it has many other uses besides this one.
> 
> So I might be missing something, but what generic uses does it have, 
> beyond structures that have some rare size related weirdness, such as 
> alignment attributes? In 99% of the cases:
> 
>    sizeof(struct) == offsetofend(struct, last_member)
> 
> right?

The idea in the vfio code is to allow the structure to evolve over time
while maintaining compatibility.  We effectively create a header in the
structure with the base functionality and flags and structure size can
tell us what optional fields are present.  A better examples is
drivers/vfio/pci/vfio_pci.c:vfio_pci_ioctl(), particularly
VFIO_DEVICE_SET_IRQS where we read what's being provided in the "header"
and then read beyond the minimum structure size if directed.  Thanks,

Alex

> > Nevertheless, I can resend a version where comment survives if you 
> > want...
> 
> So maybe extend it to a description that you think describes its uses 
> correctly? People will keep wondering about when to use this.
> 
> Thanks,
> 
> 	Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 15:30       ` Andy Lutomirski
@ 2015-03-09 15:45         ` Ingo Molnar
  0 siblings, 0 replies; 16+ messages in thread
From: Ingo Molnar @ 2015-03-09 15:45 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Denys Vlasenko, Denys Vlasenko, Linus Torvalds, Steven Rostedt,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	X86 ML, Linux Kernel Mailing List


* Andy Lutomirski <luto@amacapital.net> wrote:

> > So I might be missing something, but what generic uses does it 
> > have, beyond structures that have some rare size related 
> > weirdness, such as alignment attributes? In 99% of the cases:
> >
> >    sizeof(struct) == offsetofend(struct, last_member)
> >
> > right?
> 
> struct foo {
>     u64 a;
>     char b;
> };
> 
> sizeof(struct foo) will be 16, but offsetofend(struct foo, b) will be
> 9 on most platforms, right?

I knew I missed something obvious :-)

Let me attempt to get it right:

When the next byte after the last member of a structure is not aligned 
to the largest alignment requirement of any structure member, then the 
structure grows (is padded) and offsetofend() < sizeof().

'packed' or 'aligned' attributes will modify the largest alignment 
requirement value so they are a common but not only mechanism for this 
to be the case.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] move offsetofend() from vfio.h to stddef.h
  2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
  2015-03-09 14:58 ` [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Ingo Molnar
@ 2015-03-09 16:16 ` Linus Torvalds
  2015-03-16 12:10 ` [tip:x86/asm] include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header tip-bot for Denys Vlasenko
  2015-03-17  8:46 ` tip-bot for Denys Vlasenko
  4 siblings, 0 replies; 16+ messages in thread
From: Linus Torvalds @ 2015-03-09 16:16 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Andy Lutomirski, Steven Rostedt, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook,
	the arch/x86 maintainers, Linux Kernel Mailing List

On Mon, Mar 9, 2015 at 7:52 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> Suggested by Linus.

Actually, that was Andy, not me.

But I have nothing against it.

                       Linus

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
  2015-03-09 15:16   ` Andy Lutomirski
@ 2015-03-14 16:00   ` Pavel Machek
  2015-03-14 17:24     ` Brian Gerst
  2015-03-16 12:10   ` [tip:x86/asm] x86/asm/entry/32: Document the 32-bit SYSENTER " emergency stack" better tip-bot for Denys Vlasenko
  2015-03-17  8:46   ` tip-bot for Denys Vlasenko
  3 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2015-03-14 16:00 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Andy Lutomirski, Linus Torvalds, Steven Rostedt, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

Hi!

>  arch/x86/kernel/asm-offsets_32.c | 2 +-
>  arch/x86/kernel/cpu/common.c     | 3 ++-
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 48a61c1..9e65cf8 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -276,9 +276,9 @@ struct tss_struct {
>  	unsigned long		io_bitmap[IO_BITMAP_LONGS + 1];
>  
>  	/*
> -	 * .. and then another 0x100 bytes for the emergency kernel stack:
> +	 * and then space for temporary SYSENTER stack:
>  	 */
> -	unsigned long		stack[64];
> +	unsigned long		SYSENTER_stack[64];

Would it make sense to make it unsigned char ...stack[256], like the comment said?

Or is it supposed to be different size on 64bit, and comment was wrong?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented
  2015-03-14 16:00   ` Pavel Machek
@ 2015-03-14 17:24     ` Brian Gerst
  0 siblings, 0 replies; 16+ messages in thread
From: Brian Gerst @ 2015-03-14 17:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Denys Vlasenko, Andy Lutomirski, Linus Torvalds, Steven Rostedt,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	the arch/x86 maintainers, Linux Kernel Mailing List

On Sat, Mar 14, 2015 at 12:00 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>>  arch/x86/kernel/asm-offsets_32.c | 2 +-
>>  arch/x86/kernel/cpu/common.c     | 3 ++-
>>  3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>> index 48a61c1..9e65cf8 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -276,9 +276,9 @@ struct tss_struct {
>>       unsigned long           io_bitmap[IO_BITMAP_LONGS + 1];
>>
>>       /*
>> -      * .. and then another 0x100 bytes for the emergency kernel stack:
>> +      * and then space for temporary SYSENTER stack:
>>        */
>> -     unsigned long           stack[64];
>> +     unsigned long           SYSENTER_stack[64];
>
> Would it make sense to make it unsigned char ...stack[256], like the comment said?
>
> Or is it supposed to be different size on 64bit, and comment was wrong?

It isn't used on 64-bit.  MSR_IA32_SYSENTER_ESP is set to 0, and
tss->sp0 is read from the percpu segment instead of relative to the
stack pointer.

--
Brian Gerst

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip:x86/asm] include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header
  2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
                   ` (2 preceding siblings ...)
  2015-03-09 16:16 ` Linus Torvalds
@ 2015-03-16 12:10 ` tip-bot for Denys Vlasenko
  2015-03-17  8:46 ` tip-bot for Denys Vlasenko
  4 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-16 12:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bp, ast, keescook, wad, rostedt, luto, linux-kernel, tglx, hpa,
	mingo, torvalds, dvlasenk, oleg, fweisbec

Commit-ID:  23ec13ebc0426fc5f3ecc623c432d57f919e1b28
Gitweb:     http://git.kernel.org/tip/23ec13ebc0426fc5f3ecc623c432d57f919e1b28
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 9 Mar 2015 15:52:17 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 16 Mar 2015 11:05:37 +0100

include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header

Suggested by Andy.

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1425912738-559-1-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/stddef.h |  9 +++++++++
 include/linux/vfio.h   | 13 -------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index f4aec0e..076af43 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -19,3 +19,12 @@ enum {
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 #endif
 #endif
+
+/**
+ * offsetofend(TYPE, MEMBER)
+ *
+ * @TYPE: The type of the structure
+ * @MEMBER: The member within the structure to get the end offset of
+ */
+#define offsetofend(TYPE, MEMBER) \
+	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 2d67b89..049b2f4 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -78,19 +78,6 @@ extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
 extern void vfio_unregister_iommu_driver(
 				const struct vfio_iommu_driver_ops *ops);
 
-/**
- * offsetofend(TYPE, MEMBER)
- *
- * @TYPE: The type of the structure
- * @MEMBER: The member within the structure to get the end offset of
- *
- * Simple helper macro for dealing with variable sized structures passed
- * from user space.  This allows us to easily determine if the provided
- * structure is sized to include various fields.
- */
-#define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
-
 /*
  * External user API
  */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip:x86/asm] x86/asm/entry/32: Document the 32-bit SYSENTER " emergency stack" better
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
  2015-03-09 15:16   ` Andy Lutomirski
  2015-03-14 16:00   ` Pavel Machek
@ 2015-03-16 12:10   ` tip-bot for Denys Vlasenko
  2015-03-17  8:46   ` tip-bot for Denys Vlasenko
  3 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-16 12:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, wad, luto, keescook, rostedt, torvalds, hpa, mingo,
	linux-kernel, dvlasenk, bp, ast, fweisbec, tglx

Commit-ID:  9b41236f1032623ce0470481e97604fb5c5681fa
Gitweb:     http://git.kernel.org/tip/9b41236f1032623ce0470481e97604fb5c5681fa
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 9 Mar 2015 15:52:18 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 16 Mar 2015 11:05:37 +0100

x86/asm/entry/32: Document the 32-bit SYSENTER "emergency stack" better

Before the patch, the 'tss_struct::stack' field was not referenced anywhere.

It was used only to set SYSENTER's stack to point after the last byte
of tss_struct, thus the trailing field, stack[64], was used.

But grep would not know it. You can comment it out, compile,
and kernel will even run until an unlucky NMI corrupts
io_bitmap[] (which is also not easily detectable).

This patch changes code so that the purpose and usage of this
field is not mysterious anymore, and can be easily grepped for.

This does change generated code, for a subtle reason:
since tss_struct is ____cacheline_aligned, there happens to be
5 longs of padding at the end. Old code was using the padding
too; new code will strictly use it only for SYSENTER_stack[].

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1425912738-559-2-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/processor.h | 4 ++--
 arch/x86/kernel/asm-offsets_32.c | 2 +-
 arch/x86/kernel/cpu/common.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index d157f94..5720997 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -291,9 +291,9 @@ struct tss_struct {
 	unsigned long		io_bitmap[IO_BITMAP_LONGS + 1];
 
 	/*
-	 * .. and then another 0x100 bytes for the emergency kernel stack:
+	 * Space for the temporary SYSENTER stack:
 	 */
-	unsigned long		stack[64];
+	unsigned long		SYSENTER_stack[64];
 
 } ____cacheline_aligned;
 
diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
index 3b3b9d3..47703ae 100644
--- a/arch/x86/kernel/asm-offsets_32.c
+++ b/arch/x86/kernel/asm-offsets_32.c
@@ -68,7 +68,7 @@ void foo(void)
 
 	/* Offset from the sysenter stack to tss.sp0 */
 	DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) -
-		 sizeof(struct tss_struct));
+	       offsetofend(struct tss_struct, SYSENTER_stack));
 
 #if defined(CONFIG_LGUEST) || defined(CONFIG_LGUEST_GUEST) || defined(CONFIG_LGUEST_MODULE)
 	BLANK();
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7634833..7a3dfb1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -987,7 +987,7 @@ void enable_sep_cpu(void)
 	}
 
 	tss->x86_tss.ss1 = __KERNEL_CS;
-	tss->x86_tss.sp1 = sizeof(struct tss_struct) + (unsigned long) tss;
+	tss->x86_tss.sp1 = (unsigned long)tss + offsetofend(struct tss_struct, SYSENTER_stack);
 	wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 	wrmsr(MSR_IA32_SYSENTER_ESP, tss->x86_tss.sp1, 0);
 	wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long) ia32_sysenter_target, 0);

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip:x86/asm] include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header
  2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
                   ` (3 preceding siblings ...)
  2015-03-16 12:10 ` [tip:x86/asm] include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header tip-bot for Denys Vlasenko
@ 2015-03-17  8:46 ` tip-bot for Denys Vlasenko
  4 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-17  8:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, wad, dvlasenk, hpa, keescook, ast, rostedt, linux-kernel,
	torvalds, bp, luto, tglx, mingo, fweisbec

Commit-ID:  3876488444e71238e287459c39d7692b6f718c3e
Gitweb:     http://git.kernel.org/tip/3876488444e71238e287459c39d7692b6f718c3e
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 9 Mar 2015 15:52:17 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 17 Mar 2015 09:25:28 +0100

include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header

Suggested by Andy.

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1425912738-559-1-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/stddef.h |  9 +++++++++
 include/linux/vfio.h   | 13 -------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index f4aec0e..076af43 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -19,3 +19,12 @@ enum {
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 #endif
 #endif
+
+/**
+ * offsetofend(TYPE, MEMBER)
+ *
+ * @TYPE: The type of the structure
+ * @MEMBER: The member within the structure to get the end offset of
+ */
+#define offsetofend(TYPE, MEMBER) \
+	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 2d67b89..049b2f4 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -78,19 +78,6 @@ extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
 extern void vfio_unregister_iommu_driver(
 				const struct vfio_iommu_driver_ops *ops);
 
-/**
- * offsetofend(TYPE, MEMBER)
- *
- * @TYPE: The type of the structure
- * @MEMBER: The member within the structure to get the end offset of
- *
- * Simple helper macro for dealing with variable sized structures passed
- * from user space.  This allows us to easily determine if the provided
- * structure is sized to include various fields.
- */
-#define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof(((TYPE *)0)->MEMBER))
-
 /*
  * External user API
  */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip:x86/asm] x86/asm/entry/32: Document the 32-bit SYSENTER " emergency stack" better
  2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
                     ` (2 preceding siblings ...)
  2015-03-16 12:10   ` [tip:x86/asm] x86/asm/entry/32: Document the 32-bit SYSENTER " emergency stack" better tip-bot for Denys Vlasenko
@ 2015-03-17  8:46   ` tip-bot for Denys Vlasenko
  3 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-17  8:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: keescook, wad, hpa, oleg, torvalds, dvlasenk, tglx, linux-kernel,
	fweisbec, rostedt, ast, luto, bp, mingo

Commit-ID:  d828c71fba8922b116b4ec56c3e5bca8c822d5ae
Gitweb:     http://git.kernel.org/tip/d828c71fba8922b116b4ec56c3e5bca8c822d5ae
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Mon, 9 Mar 2015 15:52:18 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 17 Mar 2015 09:25:29 +0100

x86/asm/entry/32: Document the 32-bit SYSENTER "emergency stack" better

Before the patch, the 'tss_struct::stack' field was not referenced anywhere.

It was used only to set SYSENTER's stack to point after the last byte
of tss_struct, thus the trailing field, stack[64], was used.

But grep would not know it. You can comment it out, compile,
and kernel will even run until an unlucky NMI corrupts
io_bitmap[] (which is also not easily detectable).

This patch changes code so that the purpose and usage of this
field is not mysterious anymore, and can be easily grepped for.

This does change generated code, for a subtle reason:
since tss_struct is ____cacheline_aligned, there happens to be
5 longs of padding at the end. Old code was using the padding
too; new code will strictly use it only for SYSENTER_stack[].

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1425912738-559-2-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/processor.h | 4 ++--
 arch/x86/kernel/asm-offsets_32.c | 2 +-
 arch/x86/kernel/cpu/common.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 6a5c0ec..5abd9a5 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -291,9 +291,9 @@ struct tss_struct {
 	unsigned long		io_bitmap[IO_BITMAP_LONGS + 1];
 
 	/*
-	 * .. and then another 0x100 bytes for the emergency kernel stack:
+	 * Space for the temporary SYSENTER stack:
 	 */
-	unsigned long		stack[64];
+	unsigned long		SYSENTER_stack[64];
 
 } ____cacheline_aligned;
 
diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
index 3b3b9d3..47703ae 100644
--- a/arch/x86/kernel/asm-offsets_32.c
+++ b/arch/x86/kernel/asm-offsets_32.c
@@ -68,7 +68,7 @@ void foo(void)
 
 	/* Offset from the sysenter stack to tss.sp0 */
 	DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) -
-		 sizeof(struct tss_struct));
+	       offsetofend(struct tss_struct, SYSENTER_stack));
 
 #if defined(CONFIG_LGUEST) || defined(CONFIG_LGUEST_GUEST) || defined(CONFIG_LGUEST_MODULE)
 	BLANK();
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7634833..7a3dfb1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -987,7 +987,7 @@ void enable_sep_cpu(void)
 	}
 
 	tss->x86_tss.ss1 = __KERNEL_CS;
-	tss->x86_tss.sp1 = sizeof(struct tss_struct) + (unsigned long) tss;
+	tss->x86_tss.sp1 = (unsigned long)tss + offsetofend(struct tss_struct, SYSENTER_stack);
 	wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 	wrmsr(MSR_IA32_SYSENTER_ESP, tss->x86_tss.sp1, 0);
 	wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long) ia32_sysenter_target, 0);

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-03-17  8:47 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 14:52 [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Denys Vlasenko
2015-03-09 14:52 ` [PATCH 2/2 v2] x86: make 32-bit "emergency stack" better documented Denys Vlasenko
2015-03-09 15:16   ` Andy Lutomirski
2015-03-14 16:00   ` Pavel Machek
2015-03-14 17:24     ` Brian Gerst
2015-03-16 12:10   ` [tip:x86/asm] x86/asm/entry/32: Document the 32-bit SYSENTER " emergency stack" better tip-bot for Denys Vlasenko
2015-03-17  8:46   ` tip-bot for Denys Vlasenko
2015-03-09 14:58 ` [PATCH 1/2] move offsetofend() from vfio.h to stddef.h Ingo Molnar
2015-03-09 15:15   ` Denys Vlasenko
2015-03-09 15:28     ` Ingo Molnar
2015-03-09 15:30       ` Andy Lutomirski
2015-03-09 15:45         ` Ingo Molnar
2015-03-09 15:44       ` Alex Williamson
2015-03-09 16:16 ` Linus Torvalds
2015-03-16 12:10 ` [tip:x86/asm] include/stddef.h: Move offsetofend() from vfio.h to a generic kernel header tip-bot for Denys Vlasenko
2015-03-17  8:46 ` tip-bot for Denys Vlasenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).