From b20cffafd3a5430b1df20cdcd602d965bf7abbaa Mon Sep 17 00:00:00 2001 From: jca Date: Wed, 29 Jun 2022 12:17:31 +0000 Subject: [PATCH] Unlock the pledge(2) system call Protect the ps_pledge/ps_execpledge fields with ps_mtx. Shuffle the code to call unveil_destroy() outside the critical section. Only writes to those fields are protected. Since we may only remove bits from those fields, garbage values should do no harm even when a read crosses a write on 32 bits systems. Input claudio@ kettenis@ deraadt@, ok deraadt@ --- sys/kern/init_sysent.c | 6 ++--- sys/kern/kern_pledge.c | 56 ++++++++++++++++++++++++++++------------ sys/kern/syscalls.c | 4 +-- sys/kern/syscalls.master | 4 +-- sys/sys/proc.h | 6 ++--- sys/sys/syscall.h | 4 +-- sys/sys/syscallargs.h | 4 +-- 7 files changed, 54 insertions(+), 30 deletions(-) diff --git a/sys/kern/init_sysent.c b/sys/kern/init_sysent.c index 0b8b0fd31f2..b4f6c8149e7 100644 --- a/sys/kern/init_sysent.c +++ b/sys/kern/init_sysent.c @@ -1,10 +1,10 @@ -/* $OpenBSD: init_sysent.c,v 1.238 2022/06/27 14:26:05 cheloha Exp $ */ +/* $OpenBSD: init_sysent.c,v 1.239 2022/06/29 12:17:31 jca Exp $ */ /* * System call switch table. * * DO NOT EDIT-- this file is automatically generated. - * created from; OpenBSD: syscalls.master,v 1.224 2022/05/16 07:36:04 mvs Exp + * created from; OpenBSD: syscalls.master,v 1.225 2022/06/27 14:26:05 cheloha Exp */ #include @@ -248,7 +248,7 @@ const struct sysent sysent[] = { sys_listen }, /* 106 = listen */ { 4, s(struct sys_chflagsat_args), 0, sys_chflagsat }, /* 107 = chflagsat */ - { 2, s(struct sys_pledge_args), 0, + { 2, s(struct sys_pledge_args), SY_NOLOCK | 0, sys_pledge }, /* 108 = pledge */ { 4, s(struct sys_ppoll_args), 0, sys_ppoll }, /* 109 = ppoll */ diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c index 7f554108097..c09d5342d1c 100644 --- a/sys/kern/kern_pledge.c +++ b/sys/kern/kern_pledge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_pledge.c,v 1.283 2022/06/29 12:01:22 jca Exp $ */ +/* $OpenBSD: kern_pledge.c,v 1.284 2022/06/29 12:17:31 jca Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -465,13 +466,26 @@ sys_pledge(struct proc *p, void *v, register_t *retval) struct process *pr = p->p_p; uint64_t promises, execpromises; int error; + int unveil_cleanup = 0; + /* Check for any error in user input */ if (SCARG(uap, promises)) { error = parsepledges(p, "pledgereq", SCARG(uap, promises), &promises); if (error) return (error); + } + if (SCARG(uap, execpromises)) { + error = parsepledges(p, "pledgeexecreq", + SCARG(uap, execpromises), &execpromises); + if (error) + return (error); + } + + mtx_enter(&pr->ps_mtx); + /* Check for any error wrt current promises */ + if (SCARG(uap, promises)) { /* In "error" mode, ignore promise increase requests, * but accept promise decrease requests */ if (ISSET(pr->ps_flags, PS_PLEDGE) && @@ -480,37 +494,47 @@ sys_pledge(struct proc *p, void *v, register_t *retval) /* Only permit reductions */ if (ISSET(pr->ps_flags, PS_PLEDGE) && - (((promises | pr->ps_pledge) != pr->ps_pledge))) + (((promises | pr->ps_pledge) != pr->ps_pledge))) { + mtx_leave(&pr->ps_mtx); return (EPERM); + } } if (SCARG(uap, execpromises)) { - error = parsepledges(p, "pledgeexecreq", - SCARG(uap, execpromises), &execpromises); - if (error) - return (error); - /* Only permit reductions */ if (ISSET(pr->ps_flags, PS_EXECPLEDGE) && - (((execpromises | pr->ps_execpledge) != pr->ps_execpledge))) + (((execpromises | pr->ps_execpledge) != pr->ps_execpledge))) { + mtx_leave(&pr->ps_mtx); return (EPERM); + } } + /* Set up promises */ if (SCARG(uap, promises)) { pr->ps_pledge = promises; atomic_setbits_int(&pr->ps_flags, PS_PLEDGE); - /* - * Kill off unveil and drop unveil vnode refs if we no - * longer are holding any path-accessing pledge - */ + if ((pr->ps_pledge & (PLEDGE_RPATH | PLEDGE_WPATH | PLEDGE_CPATH | PLEDGE_DPATH | PLEDGE_TMPPATH | PLEDGE_EXEC | PLEDGE_UNIX | PLEDGE_UNVEIL)) == 0) - unveil_destroy(pr); + unveil_cleanup = 1; } if (SCARG(uap, execpromises)) { pr->ps_execpledge = execpromises; atomic_setbits_int(&pr->ps_flags, PS_EXECPLEDGE); } + + mtx_leave(&pr->ps_mtx); + + if (unveil_cleanup) { + /* + * Kill off unveil and drop unveil vnode refs if we no + * longer are holding any path-accessing pledge + */ + KERNEL_LOCK(); + unveil_destroy(pr); + KERNEL_UNLOCK(); + } + return (0); } @@ -704,11 +728,11 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) * XXX * The current hack for YP support in "getpw" * is to enable some "inet" features until - * next pledge call. Setting a bit in ps_pledge - * is not safe with respect to multiple threads, - * a very different approach is needed. + * next pledge call. */ + mtx_enter(&p->p_p->ps_mtx); p->p_p->ps_pledge |= PLEDGE_YPACTIVE; + mtx_leave(&p->p_p->ps_mtx); ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); } diff --git a/sys/kern/syscalls.c b/sys/kern/syscalls.c index 5f401c65e0c..1ed2a68689b 100644 --- a/sys/kern/syscalls.c +++ b/sys/kern/syscalls.c @@ -1,10 +1,10 @@ -/* $OpenBSD: syscalls.c,v 1.236 2022/05/16 07:38:10 mvs Exp $ */ +/* $OpenBSD: syscalls.c,v 1.237 2022/06/29 12:17:31 jca Exp $ */ /* * System call names. * * DO NOT EDIT-- this file is automatically generated. - * created from; OpenBSD: syscalls.master,v 1.224 2022/05/16 07:36:04 mvs Exp + * created from; OpenBSD: syscalls.master,v 1.225 2022/06/27 14:26:05 cheloha Exp */ const char *const syscallnames[] = { diff --git a/sys/kern/syscalls.master b/sys/kern/syscalls.master index b02d109ff27..ae187f3fdd4 100644 --- a/sys/kern/syscalls.master +++ b/sys/kern/syscalls.master @@ -1,4 +1,4 @@ -; $OpenBSD: syscalls.master,v 1.225 2022/06/27 14:26:05 cheloha Exp $ +; $OpenBSD: syscalls.master,v 1.226 2022/06/29 12:17:31 jca Exp $ ; $NetBSD: syscalls.master,v 1.32 1996/04/23 10:24:21 mycroft Exp $ ; @(#)syscalls.master 8.2 (Berkeley) 1/13/94 @@ -228,7 +228,7 @@ 106 STD { int sys_listen(int s, int backlog); } 107 STD { int sys_chflagsat(int fd, const char *path, \ u_int flags, int atflags); } -108 STD { int sys_pledge(const char *promises, \ +108 STD NOLOCK { int sys_pledge(const char *promises, \ const char *execpromises); } 109 STD { int sys_ppoll(struct pollfd *fds, \ u_int nfds, const struct timespec *ts, \ diff --git a/sys/sys/proc.h b/sys/sys/proc.h index a36fbfc7722..c5e9a228f21 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.h,v 1.331 2022/06/27 14:26:05 cheloha Exp $ */ +/* $OpenBSD: proc.h,v 1.332 2022/06/29 12:17:31 jca Exp $ */ /* $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $ */ /*- @@ -231,8 +231,8 @@ struct process { u_int32_t ps_acflag; /* Accounting flags. */ - uint64_t ps_pledge; - uint64_t ps_execpledge; + uint64_t ps_pledge; /* [m] pledge promises */ + uint64_t ps_execpledge; /* [m] execpledge promises */ int64_t ps_kbind_cookie; /* [m] */ u_long ps_kbind_addr; /* [m] */ diff --git a/sys/sys/syscall.h b/sys/sys/syscall.h index e8dc9d8fda3..2643306376f 100644 --- a/sys/sys/syscall.h +++ b/sys/sys/syscall.h @@ -1,10 +1,10 @@ -/* $OpenBSD: syscall.h,v 1.235 2022/06/27 14:26:06 cheloha Exp $ */ +/* $OpenBSD: syscall.h,v 1.236 2022/06/29 12:17:31 jca Exp $ */ /* * System call numbers. * * DO NOT EDIT-- this file is automatically generated. - * created from; OpenBSD: syscalls.master,v 1.224 2022/05/16 07:36:04 mvs Exp + * created from; OpenBSD: syscalls.master,v 1.225 2022/06/27 14:26:05 cheloha Exp */ /* syscall: "syscall" ret: "int" args: "int" "..." */ diff --git a/sys/sys/syscallargs.h b/sys/sys/syscallargs.h index daa303d8d7a..4ece1b1ea40 100644 --- a/sys/sys/syscallargs.h +++ b/sys/sys/syscallargs.h @@ -1,10 +1,10 @@ -/* $OpenBSD: syscallargs.h,v 1.238 2022/06/27 14:26:06 cheloha Exp $ */ +/* $OpenBSD: syscallargs.h,v 1.239 2022/06/29 12:17:31 jca Exp $ */ /* * System call argument lists. * * DO NOT EDIT-- this file is automatically generated. - * created from; OpenBSD: syscalls.master,v 1.224 2022/05/16 07:36:04 mvs Exp + * created from; OpenBSD: syscalls.master,v 1.225 2022/06/27 14:26:05 cheloha Exp */ #ifdef syscallarg -- 2.20.1