Skip to contentSkip to conversation

x86/fpu: Properly align size in CHECK_MEMBER_AT_END_OF macro

PATCH New Jiri Olsa 2 participants 2 messages Dec 21, 2015 – Jan 6, 2016 x86

lore
Share & tools
lore

Jiri Olsa

author
#0 lore raw

PATCH 1/1 x86/fpu: Properly align size in CHECK_MEMBER_AT_END_OF macro

The CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) checks whether MEMBER is last member of TYPE by evaluating:

offsetof(TYPE::MEMBER) + sizeof(TYPE::MEMBER) == sizeof(TYPE)

and ensuring TYPE::MEMBER is the last member of the TYPE.

This condition breaks on structs that are padded to be aligned. This patch ensures the TYPE alignment is taken into account.

This bug was revealed after adding cacheline alignment into struct sched_entity, which broke task_struct::thread check:
CHECK_MEMBER_AT_END_OF(struct task_struct, thread);

Signed-off-by Jiri Olsa <[email protected]>
---
 arch/x86/kernel/fpu/init.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/fpu/init.c +11 -2
index be39b5fde4b9..8e839e7f5e2f 100644
@@ -143,9 +143,18 @@ static void __init fpu__init_system_generic(void)
 unsigned int xstate_size;
 EXPORT_SYMBOL_GPL(xstate_size);
 
-/* Enforce that 'MEMBER' is the last field of 'TYPE': */
+/* Get alignment of the TYPE. */
+#define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test)
+
+/*
+ * Enforce that 'MEMBER' is the last field of 'TYPE'.
+ *
+ * Align the computed size with alignment of the TYPE,
+ * because that's how C aligns structs.
+ */
 #define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) \
-	BUILD_BUG_ON(sizeof(TYPE) != offsetofend(TYPE, MEMBER))
+	BUILD_BUG_ON(sizeof(TYPE) != ALIGN(offsetofend(TYPE, MEMBER), \
+					   TYPE_ALIGN(TYPE)))
 
 /*
  * We append the 'struct fpu' to the task_struct:
Signature · 1 line
2.4.3
merge bot
#1 lore raw

[tip:sched/core] x86/fpu: Properly align size in CHECK_MEMBER_AT_END_OF() macro

x86/fpu: Properly align size in CHECK_MEMBER_AT_END_OF() macro
Commit-ID:  25ec02f2c14466a4549c5dcc044b628c2cc46fde
Gitweb:     http://git.kernel.org/tip/25ec02f2c14466a4549c5dcc044b628c2cc46fde
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 21 Dec 2015 15:25:30 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 6 Jan 2016 11:06:06 +0100

x86/fpu: Properly align size in CHECK_MEMBER_AT_END_OF() macro

The CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) checks whether MEMBER is last member of TYPE by evaluating:

offsetof(TYPE::MEMBER) + sizeof(TYPE::MEMBER) == sizeof(TYPE)

and ensuring TYPE::MEMBER is the last member of the TYPE.

This condition breaks on structs that are padded to be aligned. This patch ensures the TYPE alignment is taken into account.

This bug was revealed after adding cacheline alignment into struct sched_entity, which broke task_struct::thread check:

  CHECK_MEMBER_AT_END_OF(struct task_struct, thread);
Signed-off-by Jiri Olsa <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/kernel/fpu/init.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
arch/x86/kernel/fpu/init.c +11 -2
index be39b5f..8e839e7 100644
@@ -143,9 +143,18 @@ static void __init fpu__init_system_generic(void)
 unsigned int xstate_size;
 EXPORT_SYMBOL_GPL(xstate_size);
 
-/* Enforce that 'MEMBER' is the last field of 'TYPE': */
+/* Get alignment of the TYPE. */
+#define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test)
+
+/*
+ * Enforce that 'MEMBER' is the last field of 'TYPE'.
+ *
+ * Align the computed size with alignment of the TYPE,
+ * because that's how C aligns structs.
+ */
 #define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) \
-	BUILD_BUG_ON(sizeof(TYPE) != offsetofend(TYPE, MEMBER))
+	BUILD_BUG_ON(sizeof(TYPE) != ALIGN(offsetofend(TYPE, MEMBER), \
+					   TYPE_ALIGN(TYPE)))
 
 /*
  * We append the 'struct fpu' to the task_struct: