Skip to contentSkip to conversation

Make AT_VECTOR_SIZE_ARCH 2 for x86-32

RFC PATCH In review Serge E. Hallyn 2 participants 5 messages Feb 8, 2010 – Feb 9, 2010 4 messages not in the archive x86

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 Make AT_VECTOR_SIZE_ARCH 2 for x86-32

[ RFC: Am I on crack? ]

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32, which defines two saved_auxv entries. But system.h only defines AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for CONFIG_X86_32. Fix that.

---
 arch/x86/include/asm/system.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/system.h +2 -2
index ecb544e..e04740f 100644
@@ -11,9 +11,9 @@
 #include <linux/irqflags.h>
 
 /* entries in ARCH_DLINFO: */
-#ifdef CONFIG_IA32_EMULATION
+#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
 # define AT_VECTOR_SIZE_ARCH 2
-#else
+#else /* else it's non-compat x86-64 */
 # define AT_VECTOR_SIZE_ARCH 1
 #endif
 
Signature · 1 line
1.6.0.4

A message that is not in the archive (sent off-list or not yet received): 20100208203440.GA27389-r/[email protected]

#2 lore raw
Quoted 6 lines · Serge E. Hallyn

[ RFC: Am I on crack? ]

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32, which defines two saved_auxv entries. But system.h only defines AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for CONFIG_X86_32. Fix that.

To be clear, this patch if right would be for pushing upstream immediately. It still leaves open the question of what we want to do about saved_auxv. We currently just write it out as a buffer, but since it is actually an array of longs, and therefore differently sized on x86-32 and x86-64-compat, we would need to write them out entry-by-entry (and validate no overflows for TIF_IA32 tasks). Does that seem warranted?

Quoted 28 lines
---
 arch/x86/include/asm/system.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/system.h +2 -2
index ecb544e..e04740f 100644
@@ -11,9 +11,9 @@
 #include <linux/irqflags.h>
 
 /* entries in ARCH_DLINFO: */
-#ifdef CONFIG_IA32_EMULATION
+#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
 # define AT_VECTOR_SIZE_ARCH 2
-#else
+#else /* else it's non-compat x86-64 */
 # define AT_VECTOR_SIZE_ARCH 1
 #endif
 
Signature · 6 lines
1.6.0.4

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20100208204837.GA27904-r/[email protected]

#4 lore raw
Quoted 8 lines · Serge E. Hallyn
Quoted 6 lines · Serge E. Hallyn

[ RFC: Am I on crack? ]

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32, which defines two saved_auxv entries. But system.h only defines AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for CONFIG_X86_32. Fix that.

Quoted 7 lines

To be clear, this patch if right would be for pushing upstream immediately. It still leaves open the question of what we want to do about saved_auxv. We currently just write it out as a buffer, but since it is actually an array of longs, and therefore differently sized on x86-32 and x86-64-compat, we would need to write them out entry-by-entry (and validate no overflows for TIF_IA32 tasks). Does that seem warranted?

Yes: iterate over entries and copy them.

From a brief look at the code, I don't think the contents of the
saved_auxv is used anywhere inside the kernel (it's exported via /proc), except for the reliance on a trailing AT_NULL record which is easy to test for.

Would it be wrong or insecure to export whatever garbage the user may have put in that array (assuming it is null terminated) ?

Oren.

Quoted 34 lines
Quoted 29 lines
---
 arch/x86/include/asm/system.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/system.h +2 -2
index ecb544e..e04740f 100644
@@ -11,9 +11,9 @@
 #include <linux/irqflags.h>
 
 /* entries in ARCH_DLINFO: */
-#ifdef CONFIG_IA32_EMULATION
+#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
 # define AT_VECTOR_SIZE_ARCH 2
-#else
+#else /* else it's non-compat x86-64 */
 # define AT_VECTOR_SIZE_ARCH 1
 #endif
 
Signature · 6 lines
1.6.0.4

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 21 lines · Oren Laadan
Quoted 8 lines · Serge E. Hallyn
Quoted 6 lines · Serge E. Hallyn

[ RFC: Am I on crack? ]

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32, which defines two saved_auxv entries. But system.h only defines AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for CONFIG_X86_32. Fix that.

Quoted 7 lines

To be clear, this patch if right would be for pushing upstream immediately. It still leaves open the question of what we want to do about saved_auxv. We currently just write it out as a buffer, but since it is actually an array of longs, and therefore differently sized on x86-32 and x86-64-compat, we would need to write them out entry-by-entry (and validate no overflows for TIF_IA32 tasks). Does that seem warranted?

Yes: iterate over entries and copy them.

Quoted 7 lines

From a brief look at the code, I don't think the contents of the saved_auxv is used anywhere inside the kernel (it's exported via /proc), except for the reliance on a trailing AT_NULL record which is easy to test for.

Would it be wrong or insecure to export whatever garbage the user may have put in that array (assuming it is null terminated) ?

I don't know which tools use the /proc/$$/auxv output, but I don't see why it would be unsafe so long as we (as I do) only copy AT_VECTOR_SIZE unsigned longs.

I suppose we could try and be more knowledgable about the internals and restore them to values that make sense, using code we'd share with fs/binfmt_elf.c...

-serge

A message that is not in the archive (sent off-list or not yet received): 20100209020720.GB13571-r/[email protected]

#8 lore raw
Quoted 27 lines · Serge E. Hallyn
Quoted 18 lines · Oren Laadan
Quoted 7 lines · Serge E. Hallyn
Quoted 6 lines · Serge E. Hallyn

[ RFC: Am I on crack? ]

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32, which defines two saved_auxv entries. But system.h only defines AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for CONFIG_X86_32. Fix that.

Quoted 7 lines

To be clear, this patch if right would be for pushing upstream immediately. It still leaves open the question of what we want to do about saved_auxv. We currently just write it out as a buffer, but since it is actually an array of longs, and therefore differently sized on x86-32 and x86-64-compat, we would need to write them out entry-by-entry (and validate no overflows for TIF_IA32 tasks). Does that seem warranted?

Yes: iterate over entries and copy them.

Quoted 7 lines

From a brief look at the code, I don't think the contents of the saved_auxv is used anywhere inside the kernel (it's exported via /proc), except for the reliance on a trailing AT_NULL record which is easy to test for.

Would it be wrong or insecure to export whatever garbage the user may have put in that array (assuming it is null terminated) ?

Quoted 7 lines

I don't know which tools use the /proc/$$/auxv output, but I don't see why it would be unsafe so long as we (as I do) only copy AT_VECTOR_SIZE unsigned longs.

I suppose we could try and be more knowledgable about the internals and restore them to values that make sense, using code we'd share with fs/binfmt_elf.c...

I'm quite certain that restart much validate at least that the array ends with an AT_NULL. See for example fill_auxv_note() in binmft_elf.c.

Oren.