Re: PaX MPROTECT gdb and software breakpoints

[Christos Zoulas]

On May 22,  3:42pm, rhia...@falu.nl (Rhialto) wrote:
-- Subject: Re: PaX MPROTECT gdb and software breakpoints

Here's the sysctl way.

christos

Index: kern/kern_pax.c
===
RCS file: /cvsroot/src/sys/kern/kern_pax.c,v
retrieving revision 1.49
diff -u -u -r1.49 kern_pax.c
--- kern/kern_pax.c 22 May 2016 14:26:09 -  1.49
+++ kern/kern_pax.c 22 May 2016 15:08:55 -
@@ -117,6 +117,7 @@
 #ifdef PAX_MPROTECT
 static int pax_mprotect_enabled = 1;
 static int pax_mprotect_global = PAX_MPROTECT;
+static int pax_mprotect_ptrace = 0;
 static bool pax_mprotect_elf_flags_active(uint32_t);
 #endif /* PAX_MPROTECT */
 #ifdef PAX_MPROTECT_DEBUG
@@ -205,6 +206,14 @@
"all processes."),
   NULL, 0, _mprotect_global, 0,
   CTL_CREATE, CTL_EOL);
+   sysctl_createv(clog, 0, , NULL,
+  CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
+  CTLTYPE_INT, "ptrace",
+  SYSCTL_DESCR("When enabled, allow ptrace(2) to "
+   "override protect permissions on traced "
+   "processes"),
+  NULL, 0, _mprotect_ptrace, 0,
+  CTL_CREATE, CTL_EOL);
 #ifdef PAX_MPROTECT_DEBUG
sysctl_createv(clog, 0, , NULL,
   CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
@@ -432,6 +441,24 @@
*maxprot &= ~VM_PROT_WRITE;
}
 }
+
+/*
+ * Bypass MPROTECT for traced processes
+ */
+int
+pax_mprotect_prot(struct lwp *l)
+{
+   uint32_t flags;
+
+   flags = l->l_proc->p_pax;
+   if (!pax_flags_active(flags, P_PAX_MPROTECT))
+   return 0;
+   if (!pax_mprotect_ptrace)
+   return 0;
+   return UVM_EXTRACT_PROT_ALL;
+}
+
+
 #endif /* PAX_MPROTECT */
 
 #ifdef PAX_ASLR
Index: kern/kern_proc.c
===
RCS file: /cvsroot/src/sys/kern/kern_proc.c,v
retrieving revision 1.195
diff -u -u -r1.195 kern_proc.c
--- kern/kern_proc.c4 Apr 2016 20:47:57 -   1.195
+++ kern/kern_proc.c22 May 2016 15:08:55 -
@@ -2114,7 +2114,7 @@
auio.uio_resid = xlen;
auio.uio_rw = UIO_READ;
UIO_SETUP_SYSSPACE();
-   error = uvm_io(>vm_map, );
+   error = uvm_io(>vm_map, , 0);
if (error)
goto done;
 
Index: kern/subr_copy.c
===
RCS file: /cvsroot/src/sys/kern/subr_copy.c,v
retrieving revision 1.6
diff -u -u -r1.6 subr_copy.c
--- kern/subr_copy.c21 Apr 2015 13:17:25 -  1.6
+++ kern/subr_copy.c22 May 2016 15:08:55 -
@@ -223,7 +223,7 @@
uio.uio_resid = len;
uio.uio_rw = UIO_READ;
UIO_SETUP_SYSSPACE();
-   error = uvm_io(>vm_map, );
+   error = uvm_io(>vm_map, , 0);
 
return (error);
 }
@@ -256,7 +256,7 @@
uio.uio_resid = len;
uio.uio_rw = UIO_WRITE;
UIO_SETUP_SYSSPACE();
-   error = uvm_io(>vm_map, );
+   error = uvm_io(>vm_map, , 0);
 
return (error);
 }
Index: kern/sys_process.c
===
RCS file: /cvsroot/src/sys/kern/sys_process.c,v
retrieving revision 1.168
diff -u -u -r1.168 sys_process.c
--- kern/sys_process.c  4 Apr 2016 20:47:57 -   1.168
+++ kern/sys_process.c  22 May 2016 15:08:55 -
@@ -122,12 +122,14 @@
 
 #include "opt_ptrace.h"
 #include "opt_ktrace.h"
+#include "opt_pax.h"
 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1118,7 +1120,7 @@
mutex_exit(>vm_map.misc_lock);
if (error != 0)
return (error);
-   error = uvm_io(>vm_map, uio);
+   error = uvm_io(>vm_map, uio, pax_mprotect_prot(l));
uvmspace_free(vm);
 
 #ifdef PMAP_NEED_PROCWR
Index: sys/pax.h
===
RCS file: /cvsroot/src/sys/sys/pax.h,v
retrieving revision 1.21
diff -u -u -r1.21 pax.h
--- sys/pax.h   22 May 2016 14:26:10 -  1.21
+++ sys/pax.h   22 May 2016 15:08:55 -
@@ -67,6 +67,7 @@
 struct lwp *, vm_prot_t *, vm_prot_t *);
 #ifndef PAX_MPROTECT
 # define PAX_MPROTECT_ADJUST(a, b, c)
+# define pax_mprotect_prot(l)  0
 #else
 # ifdef PAX_MPROTECT_DEBUG
 #  define PAX_MPROTECT_ADJUST(a, b, c) \
@@ -75,6 +76,7 @@
 #  define PAX_MPROTECT_ADJUST(a, b, c) \
 pax_mprotect_adjust((a), (b), (c))
 # endif
+extern int pax_mprotect_prot(struct lwp *);
 #endif
 int pax_segvguard(struct lwp *, struct vnode *, const char *, bool);
 
Index: uvm/uvm_extern.h
=

Re: PaX MPROTECT gdb and software breakpoints

[Rhialto]

On Sun 22 May 2016 at 09:29:17 -0400, Christos Zoulas wrote:
> On May 22, 10:36am, bou...@antioche.eu.org (Manuel Bouyer) wrote:
> -- Subject: Re: PaX MPROTECT gdb and software breakpoints
> 
> | I occasionally need to gdb-attach to a running process (usually a daemon)
> | to try to understand what's going on. I think it's important to keep
> | the ability to attach to a running process, which has not been
> | started in a specific way before.
> 
> This only affects breakpoints; if we want to fix breakpoints, we can
> at the cost of weakening MPROTECT.

There was earlier mention of a sysctl knob (default safe) to do this.
I read that as being system-global, but it could also be per-process.
Possibly gdb could know about it or at least tell the user about it.
An entry in /proc could be relevant as well.

> christos
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert  -- Wayland: Those who don't understand X
\X/ rhialto/at/xs4all.nl-- are condemned to reinvent it. Poorly.


signature.asc
Description: PGP signature

Re: PaX MPROTECT gdb and software breakpoints

[Christos Zoulas]

On May 22, 10:36am, bou...@antioche.eu.org (Manuel Bouyer) wrote:
-- Subject: Re: PaX MPROTECT gdb and software breakpoints

| I occasionally need to gdb-attach to a running process (usually a daemon)
| to try to understand what's going on. I think it's important to keep
| the ability to attach to a running process, which has not been
| started in a specific way before.

This only affects breakpoints; if we want to fix breakpoints, we can
at the cost of weakening MPROTECT.

christos

Re: PaX MPROTECT gdb and software breakpoints

[Paul Goyette]

uOn Sun, 22 May 2016, Martin Husemann wrote:


On Sat, May 21, 2016 at 04:21:45PM -0400, Christos Zoulas wrote:

There are three solutions I can think of (maybe you can think of a better
one):

1. Leave it as it is and document that people will have to paxctl +m
   the executables that they need to mprotect before they can debug
   it.
2. create a uvm_io1() and a new UVM_EXTRACT that breaks MPROTECT and
   fetches with the right mappings. I don't like that, it breaks MPROTECT
   and uvm contracts. But it has the advantage of working on already
   started process we PT_ATTACH to.
3. when process is execed and is already traced (PT_TRACE_ME), disable
   MPROTECT. This should work with processes started from gdb, but
   not attached ones. We will still need to explain that to attach
   and insert breakpoints you need to set MPROTECT off.


Tricky. I find (3) is a bit too limited, but it is the only clean way.
Like David suggested, (1) could be extended by a gdb message explaining
the issue.

(2) is evil, but may be needed. Of course we should restrict the new
path to traced processes, and maybe even limit it to root additionally?


If we go ahead with (2), could we at least have a sysctl knob to enable 
it (with default of Disabled)?  That way, you could get the needed 
functionality, but only with an explicit action to acknowledge that you 
are breaking all the contracts...



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

Re: PaX MPROTECT gdb and software breakpoints

[Manuel Bouyer]

On Sat, May 21, 2016 at 04:21:45PM -0400, Christos Zoulas wrote:
> 
> Hello,
> 
> Now that PaX MPROTECT is the default on 3 platforms (amd64, i386, and evbarm)
> the problem of software breakpoints and gdb has surfaced. Namely if MPROTECT
> is on, and you try to set a breakpoint you get:
> 
> (gdb) break main
> Breakpoint 1 at 0x107e0: file aslr.c, line 8.
> (gdb) r
> Starting program: /u/christos/a.out 
> Warning:
> Cannot insert breakpoint 1.
> Cannot access memory at address 0x107e0
> Cannot insert breakpoint -1.
> Temporarily disabling shared library breakpoints:
> breakpoint #-1
> 
> 
> Gdb tries to write the breakpoint instruction in the text segment,
> uvm_io() calls uvm_map_extract(... | UVM_EXTRACT_FIXPROT), but the
> max protection for the text segment does not allow writes anymore.
> 
> There are three solutions I can think of (maybe you can think of a better
> one):
> 
> 1. Leave it as it is and document that people will have to paxctl +m
>the executables that they need to mprotect before they can debug
>it.
> 2. create a uvm_io1() and a new UVM_EXTRACT that breaks MPROTECT and
>fetches with the right mappings. I don't like that, it breaks MPROTECT
>and uvm contracts. But it has the advantage of working on already
>started process we PT_ATTACH to.
> 3. when process is execed and is already traced (PT_TRACE_ME), disable
>MPROTECT. This should work with processes started from gdb, but
>not attached ones. We will still need to explain that to attach
>and insert breakpoints you need to set MPROTECT off.
> 
> Opinions?

I occasionally need to gdb-attach to a running process (usually a daemon)
to try to understand what's going on. I think it's important to keep
the ability to attach to a running process, which has not been
started in a specific way before.

-- 
Manuel Bouyer 
 NetBSD: 26 ans d'experience feront toujours la difference
--

Re: PaX MPROTECT gdb and software breakpoints

[David Brownlee]

On 21 May 2016 at 21:21, Christos Zoulas  wrote:
>
> Hello,
>
> Now that PaX MPROTECT is the default on 3 platforms (amd64, i386, and evbarm)
> the problem of software breakpoints and gdb has surfaced. Namely if MPROTECT
> is on, and you try to set a breakpoint you get:
>
> (gdb) break main
> Breakpoint 1 at 0x107e0: file aslr.c, line 8.
> (gdb) r
> Starting program: /u/christos/a.out
> Warning:
> Cannot insert breakpoint 1.
> Cannot access memory at address 0x107e0
> Cannot insert breakpoint -1.
> Temporarily disabling shared library breakpoints:
> breakpoint #-1
>
>
> Gdb tries to write the breakpoint instruction in the text segment,
> uvm_io() calls uvm_map_extract(... | UVM_EXTRACT_FIXPROT), but the
> max protection for the text segment does not allow writes anymore.
>
> There are three solutions I can think of (maybe you can think of a better
> one):
>
> 1. Leave it as it is and document that people will have to paxctl +m
>the executables that they need to mprotect before they can debug
>it.
> 2. create a uvm_io1() and a new UVM_EXTRACT that breaks MPROTECT and
>fetches with the right mappings. I don't like that, it breaks MPROTECT
>and uvm contracts. But it has the advantage of working on already
>started process we PT_ATTACH to.
> 3. when process is execed and is already traced (PT_TRACE_ME), disable
>MPROTECT. This should work with processes started from gdb, but
>not attached ones. We will still need to explain that to attach
>and insert breakpoints you need to set MPROTECT off.

Possibly a variant on 1 which includes a patch to gdb to suggest this
case when the breakpoint insertion fails, so someone who has not
already read about the situation has direct information on how to
"fix" it when they hit it