ptrace "fix" breaks ia64

ptrace "fix" breaks ia64

[David Mosberger]

Roland,

Peter Chubb found that your recent ptrace change to fix x86-64 access
to the 32-bit vsyscall page breaks ia64.  See:

 http://www.gelato.unsw.edu.au/linux-ia64/0407/10253.html

The problem is due to the fact that the gate page on ia64 really does
live in the kernel-mapped segment (as your original code correctly
assumed).  Furthermore, pgd_offset_k() is different from pgd_offset()
since the kernel-mapped segment gets a full page-directory inside a
single region, whereas user-space regions get only 1/8th of a
page-directory, so it's not OK to use pgd_offset() in lieu of
pgd_offset().

As Peter's mail suggests, we _could_ make pgd_offset() smarter by
automatically redirecting it to pgd_offset_k() when necessary, but
that's not a nice solution because it would slow down everything else
and would kind of defeat the purpose of having separate pgd_offset_k()
and pgd_offset() macros.

I suppose we could have a new macro pgd_offset_gate() or something
along those lines to accommodate platform-differences in where the
gage page lives.

	--david

Re: ptrace "fix" breaks ia64

[Roland McGrath]

> The problem is due to the fact that the gate page on ia64 really does
> live in the kernel-mapped segment (as your original code correctly
> assumed).  Furthermore, pgd_offset_k() is different from pgd_offset()
> since the kernel-mapped segment gets a full page-directory inside a
> single region, whereas user-space regions get only 1/8th of a
> page-directory, so it's not OK to use pgd_offset() in lieu of
> pgd_offset().

Sorry.  I skimmed all the code and comments for pgd_offset_k and thought at
the time that it was strictly an optimized shortcut for pgd_offset.
Clearly I did not understand the ia64 code.

> I suppose we could have a new macro pgd_offset_gate() or something
> along those lines to accommodate platform-differences in where the
> gage page lives.

That seems like the reasonable thing to do.  I considered just putting all
that logic into arch-specific code, joining with the get_gate_vma code.
But that would leave x86_64 requiring duplication of the generic version.
At least with the various arch cases around now, just adding pgd_offset_gate 
is the thing that will allow maximal code sharing.


Thanks,
Roland

Re: ptrace "fix" breaks ia64

[David Mosberger]

>>>>> On Wed, 7 Jul 2004 13:47:33 -0700, Roland McGrath <roland@redhat.com> said:

  Roland> Sorry.  I skimmed all the code and comments for pgd_offset_k
  Roland> and thought at the time that it was strictly an optimized
  Roland> shortcut for pgd_offset.  Clearly I did not understand the
  Roland> ia64 code.

  >> I suppose we could have a new macro pgd_offset_gate() or
  >> something along those lines to accommodate platform-differences
  >> in where the gage page lives.

  Roland> That seems like the reasonable thing to do.  I considered
  Roland> just putting all that logic into arch-specific code, joining
  Roland> with the get_gate_vma code.  But that would leave x86_64
  Roland> requiring duplication of the generic version.  At least with
  Roland> the various arch cases around now, just adding
  Roland> pgd_offset_gate is the thing that will allow maximal code
  Roland> sharing.

The patch below fixes the problem for ia64 and should have no effect
on x86 or x86-64.  Just out of paranoia, I compiled-tested it on x86.

If it looks OK, please apply, Linus or Andrew.

Thanks,

	--david

---
Make get_user_pages() work again for ia64 gate area

Changeset

  roland@redhat.com[torvalds]|ChangeSet|20040624165002|30880

inadvertently broke ia64 because the patch assumed that pgd_offset_k()
is just an optimization of pgd_offset(), which it is not.  This patch
fixes the problem by introducing pgd_offset_gate().  On architectures
on which the gate area lives in the user's address-space, this should
be aliased to pgd_offset() and on architectures on which the gate area
lives in the kernel-mapped segment, this should be aliased to
pgd_offset_k().

Signed-off-by: davidm@hpl.hp.com

=== include/asm-generic/pgtable.h 1.6 vs edited ==--- 1.6/include/asm-generic/pgtable.h	Wed May 26 07:56:17 2004
+++ edited/include/asm-generic/pgtable.h	Wed Jul  7 18:02:20 2004
@@ -122,4 +122,8 @@
 #define page_test_and_clear_young(page) (0)
 #endif
 
+#ifndef __HAVE_ARCH_PGD_OFFSET_GATE
+#define pgd_offset_gate(mm, addr)	pgd_offset(mm, addr)
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */
=== include/asm-ia64/pgtable.h 1.43 vs edited ==--- 1.43/include/asm-ia64/pgtable.h	Sat Jun 19 07:48:59 2004
+++ edited/include/asm-ia64/pgtable.h	Wed Jul  7 18:03:55 2004
@@ -321,6 +321,11 @@
 #define pgd_offset_k(addr) \
 	(init_mm.pgd + (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)))
 
+/* Look up a pgd entry in the gate area.  On IA-64, the gate-area
+   resides in the kernel-mapped segment, hence we use pgd_offset_k()
+   here.  */
+#define pgd_offset_gate(mm, addr)	pgd_offset_k(addr)
+
 /* Find an entry in the second-level page table.. */
 #define pmd_offset(dir,addr) \
 	((pmd_t *) pgd_page(*(dir)) + (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1)))
@@ -552,6 +557,7 @@
 #define __HAVE_ARCH_PTEP_SET_WRPROTECT
 #define __HAVE_ARCH_PTEP_MKDIRTY
 #define __HAVE_ARCH_PTE_SAME
+#define __HAVE_ARCH_PGD_OFFSET_GATE
 #include <asm-generic/pgtable.h>
 
 #endif /* _ASM_IA64_PGTABLE_H */
=== mm/memory.c 1.148 vs edited ==--- 1.148/mm/memory.c	Tue Jul  6 22:19:26 2004
+++ edited/mm/memory.c	Wed Jul  7 18:02:29 2004
@@ -727,7 +727,7 @@
 			pte_t *pte;
 			if (write) /* user gate pages are read-only */
 				return i ? : -EFAULT;
-			pgd = pgd_offset(mm, pg);
+			pgd = pgd_offset_gate(mm, pg);
 			if (!pgd)
 				return i ? : -EFAULT;
 			pmd = pmd_offset(pgd, pg);

Re: ptrace "fix" breaks ia64

[David Mosberger]

[Oops, I promptly forgot to credit Peter Chubb for tracking down this bug.
 Mail below is unchanged except for the ChangeLog entry.]

>>>>> On Wed, 7 Jul 2004 13:47:33 -0700, Roland McGrath <roland@redhat.com> said:

  Roland> Sorry.  I skimmed all the code and comments for pgd_offset_k
  Roland> and thought at the time that it was strictly an optimized
  Roland> shortcut for pgd_offset.  Clearly I did not understand the
  Roland> ia64 code.

  >> I suppose we could have a new macro pgd_offset_gate() or
  >> something along those lines to accommodate platform-differences
  >> in where the gage page lives.

  Roland> That seems like the reasonable thing to do.  I considered
  Roland> just putting all that logic into arch-specific code, joining
  Roland> with the get_gate_vma code.  But that would leave x86_64
  Roland> requiring duplication of the generic version.  At least with
  Roland> the various arch cases around now, just adding
  Roland> pgd_offset_gate is the thing that will allow maximal code
  Roland> sharing.

The patch below fixes the problem for ia64 and should have no effect
on x86 or x86-64.  Just out of paranoia, I compiled-tested it on x86.

If it looks OK, please apply, Linus or Andrew.

Thanks,

	--david

---
Make get_user_pages() work again for ia64 gate area

Changeset

  roland@redhat.com[torvalds]|ChangeSet|20040624165002|30880

inadvertently broke ia64 because the patch assumed that pgd_offset_k()
is just an optimization of pgd_offset(), which it is not.  This patch
fixes the problem by introducing pgd_offset_gate().  On architectures
on which the gate area lives in the user's address-space, this should
be aliased to pgd_offset() and on architectures on which the gate area
lives in the kernel-mapped segment, this should be aliased to
pgd_offset_k().

This bug was found and tracked down by Peter Chubb.

Signed-off-by: davidm@hpl.hp.com

=== include/asm-generic/pgtable.h 1.6 vs edited ==--- 1.6/include/asm-generic/pgtable.h	Wed May 26 07:56:17 2004
+++ edited/include/asm-generic/pgtable.h	Wed Jul  7 18:02:20 2004
@@ -122,4 +122,8 @@
 #define page_test_and_clear_young(page) (0)
 #endif
 
+#ifndef __HAVE_ARCH_PGD_OFFSET_GATE
+#define pgd_offset_gate(mm, addr)	pgd_offset(mm, addr)
+#endif
+
 #endif /* _ASM_GENERIC_PGTABLE_H */
=== include/asm-ia64/pgtable.h 1.43 vs edited ==--- 1.43/include/asm-ia64/pgtable.h	Sat Jun 19 07:48:59 2004
+++ edited/include/asm-ia64/pgtable.h	Wed Jul  7 18:03:55 2004
@@ -321,6 +321,11 @@
 #define pgd_offset_k(addr) \
 	(init_mm.pgd + (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)))
 
+/* Look up a pgd entry in the gate area.  On IA-64, the gate-area
+   resides in the kernel-mapped segment, hence we use pgd_offset_k()
+   here.  */
+#define pgd_offset_gate(mm, addr)	pgd_offset_k(addr)
+
 /* Find an entry in the second-level page table.. */
 #define pmd_offset(dir,addr) \
 	((pmd_t *) pgd_page(*(dir)) + (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1)))
@@ -552,6 +557,7 @@
 #define __HAVE_ARCH_PTEP_SET_WRPROTECT
 #define __HAVE_ARCH_PTEP_MKDIRTY
 #define __HAVE_ARCH_PTE_SAME
+#define __HAVE_ARCH_PGD_OFFSET_GATE
 #include <asm-generic/pgtable.h>
 
 #endif /* _ASM_IA64_PGTABLE_H */
=== mm/memory.c 1.148 vs edited ==--- 1.148/mm/memory.c	Tue Jul  6 22:19:26 2004
+++ edited/mm/memory.c	Wed Jul  7 18:02:29 2004
@@ -727,7 +727,7 @@
 			pte_t *pte;
 			if (write) /* user gate pages are read-only */
 				return i ? : -EFAULT;
-			pgd = pgd_offset(mm, pg);
+			pgd = pgd_offset_gate(mm, pg);
 			if (!pgd)
 				return i ? : -EFAULT;
 			pmd = pmd_offset(pgd, pg);