[PATCH -mm] utrace: nommu fixup support utrace

[PATCH -mm] utrace: nommu fixup support utrace

[Wu, Bryan]

Hi folks,

When adding utrace support to blackfin architecture, I found a compiling
error in nommu related utrace stuff. Then this little patch will fix
this for nommu arch utrace.

Signed-off-by: Bryan Wu <bryan.wu@analog.com> 
---
 mm/nommu.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6/mm/nommu.c
===================================================================
--- linux-2.6.orig/mm/nommu.c	2007-03-06 15:14:22.000000000 +0800
+++ linux-2.6/mm/nommu.c	2007-03-06 15:16:01.000000000 +0800
@@ -673,7 +673,11 @@
 	 * it's being traced - otherwise breakpoints set in it may interfere
 	 * with another untraced process
 	 */
+#ifdef CONFIG_UTRACE
+	if (flags & MAP_PRIVATE) 
+#else
 	if ((flags & MAP_PRIVATE) && (current->ptrace & PT_PTRACED))
+#endif
 		vm_flags &= ~VM_MAYSHARE;
 
 	return vm_flags;
_

Thanks
-Bryan Wu

Re: [PATCH -mm] utrace: nommu fixup support utrace

[David Howells]

Wu, Bryan <bryan.wu@analog.com> wrote:

> When adding utrace support to blackfin architecture, I found a compiling
> error in nommu related utrace stuff. Then this little patch will fix
> this for nommu arch utrace.

You really don't want to do it like this.  This prevents ELF shared libraries
from being shared at all if UTRACE is enabled.

David

Re: [PATCH -mm] utrace: nommu fixup support utrace

[Wu, Bryan]

On Tue, 2007-03-06 at 09:37 +0000, David Howells wrote:
> Wu, Bryan <bryan.wu@analog.com> wrote:
> 
> > When adding utrace support to blackfin architecture, I found a compiling
> > error in nommu related utrace stuff. Then this little patch will fix
> > this for nommu arch utrace.
> 
> You really don't want to do it like this.  This prevents ELF shared libraries
> from being shared at all if UTRACE is enabled.
> 
> David

Got it, I will find another way to fix this.
Now, I just make it compile pass. 

Thanks
-Bryan

Re: [PATCH -mm] utrace: nommu fixup support utrace

[Roland McGrath]

That old ptrace check seems pretty questionable to me.  I think what you
want is for the nommu world's get_user_pages/access_process_vm when called
with force=1,write=1 on a read-only MAP_PRIVATE page to do something more
morally similar to the mmu world's COW than it does now.


Thanks,
Roland

Re: [PATCH -mm] utrace: nommu fixup support utrace

[David Howells]

Roland McGrath <roland@redhat.com> wrote:

> That old ptrace check seems pretty questionable to me.  I think what you
> want is for the nommu world's get_user_pages/access_process_vm when called
> with force=1,write=1 on a read-only MAP_PRIVATE page to do something more
> morally similar to the mmu world's COW than it does now.

Such as what?  You *can't* do COW without relocating all the pointers userspace
may have into that VMA.  However, unless you force non-sharing of R/O
MAP_PRIVATE VMAs, you will have text segments of executables and libraries
shared with other processes.  Imagine: you set a breakpoint in uclibc read()
and your whole system dies instantly.

What I did is to say that if a process has PT_TRACED set then the MAP_PRIVATE
VMAs start with their own copies.  The debugger can set this in a new process
by cloning it with appropriate CLONE_xxx flags.

It's not perfect, I know, but it's the best I could come up with as a solution
to debugging things in a NOMMU environment that supports shared libraries and
executables.

David

Re: [PATCH -mm] utrace: nommu fixup support utrace

[Roland McGrath]

I understand the NOMMU situation, and you are already screwed by
PTRACE_ATTACH.  What I meant to suggest is that I would start from a
safety point of view with get_user_pages/access_process_vm refusing to
do force&&write to MAP_PRIVATE pages that are in fact being shared
(ETXTBSY or something).  (When it's not being shared, it should do
whatever is necessary to make sure that page is known dirty and not
hand it out for later mappings.)  Then you can go about trying to make
the safe (no sharing) case come about when you want it.  You still
won't win with PTRACE_ATTACH and the like unless you happen not to
have sharing in the places you insert your breakpoints at the time.
But at least the debugger will just lose, instead of breaking
unsuspecting processes.  With the utrace patches, you can approximate
the ptrace check you had with something like:

	if (tracehook_consider_fatal_signal(current, SIGTRAP))

or whatever signal you think poking text might result in that the
debugger will be looking for (atm it doesn't actually matter what
signo you pass).  This returns true when ptrace is in use, and
probably also for later utrace-based ways a debugger attaches if it is
expecting ahead of time to be debugging heavily as with breakpoints.
(And that's about the best you can do for a single address space system.)


Thanks,
Roland

Re: [PATCH -mm] utrace: nommu fixup support utrace

[David Howells]

Roland McGrath <roland@redhat.com> wrote:

> What I meant to suggest is that I would start from a safety point of view
> with get_user_pages/access_process_vm refusing to do force&&write to
> MAP_PRIVATE pages that are in fact being shared (ETXTBSY or something).

That's a good idea.  The other possibility I've thought of is maintaining a
list of the changes made to such a region and deapplying them / reapplying
them as the processes get scheduled.  That's probably fine as long as it's
just a few breakpoints and it's a single-CPU system.

But this is irrelevant as it doesn't address the sharing-prevention issue.

> (When it's not being shared, it should do whatever is necessary to make sure
> that page is known dirty and not hand it out for later mappings.)

NOMMU doesn't with pages at this level, but deals with regions of memory
instead.  A mapping may be part of a page, a whole page, or several pages.
NOMMU private file mmap() allocates using kmalloc(), so if you allocate a
1-byte buffer, that's all you're guaranteed to get.

As it happens, when the code sees PT_PTRACED, the VMA is marked as being
unshareable by the simple expedient of turning off VM_MAYSHARE, meaning that it
neither shares with already existing mappings, nor will it be shareable by
mappings that have yet to be made - even within the same process.

> Then you can go about trying to make the safe (no sharing) case come about
> when you want it.

Which brings us back to the if-statement you objected to.  Its presence is
still required so as to prevent sharing of the executable and loader, and this
seems a good a way to do it as any as far as I can see.  Remember that it has
be controlled by something that can be set before the binfmt load_binary() op
runs.

David