Changeset View
Standalone View
sys/kern/sys_process.c
Show All 27 Lines | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | ||||
* SUCH DAMAGE. | * SUCH DAMAGE. | ||||
*/ | */ | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include "opt_compat.h" | #include "opt_compat.h" | ||||
#include "opt_pax.h" | |||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/systm.h> | #include <sys/systm.h> | ||||
#include <sys/lock.h> | #include <sys/lock.h> | ||||
#include <sys/mutex.h> | #include <sys/mutex.h> | ||||
#include <sys/syscallsubr.h> | #include <sys/syscallsubr.h> | ||||
#include <sys/sysent.h> | #include <sys/sysent.h> | ||||
#include <sys/sysproto.h> | #include <sys/sysproto.h> | ||||
#include <sys/pax.h> | |||||
#include <sys/priv.h> | #include <sys/priv.h> | ||||
#include <sys/proc.h> | #include <sys/proc.h> | ||||
#include <sys/procctl.h> | #include <sys/procctl.h> | ||||
#include <sys/vnode.h> | #include <sys/vnode.h> | ||||
#include <sys/ptrace.h> | #include <sys/ptrace.h> | ||||
#include <sys/rwlock.h> | #include <sys/rwlock.h> | ||||
#include <sys/sx.h> | #include <sys/sx.h> | ||||
#include <sys/malloc.h> | #include <sys/malloc.h> | ||||
▲ Show 20 Lines • Show All 595 Lines • ▼ Show 20 Lines | #ifdef COMPAT_FREEBSD32 | ||||
struct ptrace_lwpinfo32 *pl32 = NULL; | struct ptrace_lwpinfo32 *pl32 = NULL; | ||||
struct ptrace_lwpinfo plr; | struct ptrace_lwpinfo plr; | ||||
#endif | #endif | ||||
curp = td->td_proc; | curp = td->td_proc; | ||||
/* Lock proctree before locking the process. */ | /* Lock proctree before locking the process. */ | ||||
switch (req) { | switch (req) { | ||||
#ifdef PAX_ASLR | |||||
case PT_PAX: | |||||
#endif | |||||
imp: Doesn't need to be an ifdef. | |||||
Not Done Inline ActionsI'll remove this. I'll replace with a static inline function with a body that just returns, defined in sys/sys/pax.h lattera-gmail.com: I'll remove this. I'll replace with a static inline function with a body that just returns… | |||||
Not Done Inline ActionsI decided to go with a NULL macro instead of a static inline function. This is reflected in the new patch that was uploaded today. lattera-gmail.com: I decided to go with a NULL macro instead of a static inline function. This is reflected in the… | |||||
case PT_TRACE_ME: | case PT_TRACE_ME: | ||||
case PT_ATTACH: | case PT_ATTACH: | ||||
case PT_STEP: | case PT_STEP: | ||||
case PT_CONTINUE: | case PT_CONTINUE: | ||||
case PT_TO_SCE: | case PT_TO_SCE: | ||||
case PT_TO_SCX: | case PT_TO_SCX: | ||||
case PT_SYSCALL: | case PT_SYSCALL: | ||||
case PT_FOLLOW_FORK: | case PT_FOLLOW_FORK: | ||||
case PT_DETACH: | case PT_DETACH: | ||||
sx_xlock(&proctree_lock); | sx_xlock(&proctree_lock); | ||||
proctree_locked = 1; | proctree_locked = 1; | ||||
break; | break; | ||||
default: | default: | ||||
break; | break; | ||||
} | } | ||||
write = 0; | write = 0; | ||||
if (req == PT_TRACE_ME) { | if (req == PT_TRACE_ME || req == PT_PAX) { | ||||
Not Done Inline ActionsPT_PAX seems inconsistent with other ptrace() operations .. I wonder if this is being done right? rwatson: PT_PAX seems inconsistent with other ptrace() operations .. I wonder if this is being done… | |||||
p = td->td_proc; | p = td->td_proc; | ||||
PROC_LOCK(p); | PROC_LOCK(p); | ||||
} else { | } else { | ||||
if (pid <= PID_MAX) { | if (pid <= PID_MAX) { | ||||
if ((p = pfind(pid)) == NULL) { | if ((p = pfind(pid)) == NULL) { | ||||
if (proctree_locked) | if (proctree_locked) | ||||
sx_xunlock(&proctree_lock); | sx_xunlock(&proctree_lock); | ||||
return (ESRCH); | return (ESRCH); | ||||
▲ Show 20 Lines • Show All 50 Lines • ▼ Show 20 Lines | if (SV_PROC_FLAG(td2->td_proc, SV_ILP32)) | ||||
safe = 1; | safe = 1; | ||||
wrap32 = 1; | wrap32 = 1; | ||||
} | } | ||||
#endif | #endif | ||||
/* | /* | ||||
* Permissions check | * Permissions check | ||||
*/ | */ | ||||
switch (req) { | switch (req) { | ||||
case PT_PAX: | |||||
Not Done Inline ActionsAgain, is a MAC check needed here? imp: Again, is a MAC check needed here? | |||||
Not Done Inline ActionsNo. lattera-gmail.com: No. | |||||
/* securelevel should be 0 to allow this */ | |||||
Not Done Inline ActionsWhy disallow this at higher securelevels? rwatson: Why disallow this at higher securelevels? | |||||
Not Done Inline ActionsBecause allowing a user to disable ASLR is risky. I would like to impose some restrictions on what the user can do in regards to using an API to disable ASLR. lattera-gmail.com: Because allowing a user to disable ASLR is risky. I would like to impose some restrictions on… | |||||
Not Done Inline ActionsLots of things are risky in UNIX, but aren't controlled by securelevels. In general, securelevels limit root privileges rather than user privileges, and does so in order to protect the TCB (e.g., the kernel, raw filesystem access, etc) from manipulation that might be used to disable securelevels. It's not clear to me that this qualifies. rwatson: Lots of things are risky in UNIX, but aren't controlled by securelevels. In general… | |||||
Not Done Inline ActionsUse a sentence. rwatson: Use a sentence.
"should" -> "must". | |||||
if ((data & PAX_NOTE_NOASLR) == PAX_NOTE_NOASLR) { | |||||
if (securelevel_gt(p->p_ucred, 0) != 0) { | |||||
Not Done Inline Actionsp->p_ucred is almost never the right credential to use for access control -- do you mean td->td_ucred? You do hold PROC_LOCK(p) here, so if you mean p->p_ucred, you should have a PROC_LOCK_ASSERT() at the top of this 'case' to document the requirement for the lock. rwatson: p->p_ucred is almost never the right credential to use for access control -- do you mean td… | |||||
Not Done Inline ActionsSwitching that to use the thread should be no problem. Will address in the next patch. lattera-gmail.com: Switching that to use the thread should be no problem. Will address in the next patch. | |||||
Not Done Inline ActionsThis appears not to have been addressed. rwatson: This appears not to have been addressed. | |||||
error = EPERM; | |||||
Not Done Inline ActionsWhy not use the return value from securelevel_gt()? rwatson: Why not use the return value from securelevel_gt()? | |||||
Not Done Inline ActionsCan do. Will address in the next patch. lattera-gmail.com: Can do. Will address in the next patch. | |||||
goto fail; | |||||
} | |||||
} | |||||
break; | |||||
case PT_TRACE_ME: | case PT_TRACE_ME: | ||||
/* Always legal. */ | /* Always legal. */ | ||||
break; | break; | ||||
case PT_ATTACH: | case PT_ATTACH: | ||||
/* Self */ | /* Self */ | ||||
if (p->p_pid == td->td_proc->p_pid) { | if (p->p_pid == td->td_proc->p_pid) { | ||||
error = EINVAL; | error = EINVAL; | ||||
▲ Show 20 Lines • Show All 69 Lines • ▼ Show 20 Lines | #endif | ||||
/* | /* | ||||
* Actually do the requests | * Actually do the requests | ||||
*/ | */ | ||||
td->td_retval[0] = 0; | td->td_retval[0] = 0; | ||||
switch (req) { | switch (req) { | ||||
case PT_PAX: | |||||
p->p_pax = data; | |||||
break; | |||||
Not Done Inline ActionsAdd blank line. rwatson: Add blank line. | |||||
case PT_TRACE_ME: | case PT_TRACE_ME: | ||||
/* set my trace flag and "owner" so it can read/write me */ | /* set my trace flag and "owner" so it can read/write me */ | ||||
p->p_flag |= P_TRACED; | p->p_flag |= P_TRACED; | ||||
if (p->p_flag & P_PPWAIT) | if (p->p_flag & P_PPWAIT) | ||||
p->p_flag |= P_PPTRACE; | p->p_flag |= P_PPTRACE; | ||||
p->p_oppid = p->p_pptr->p_pid; | p->p_oppid = p->p_pptr->p_pid; | ||||
break; | break; | ||||
▲ Show 20 Lines • Show All 598 Lines • Show Last 20 Lines |
Doesn't need to be an ifdef.