Initially I just wandered in syscall_mi.h and found the locking scheme
super weird, even if technically correct. pledge_syscall() better be
safe to call without the kernel lock so I don't understand why we're
sometimes calling it with the kernel lock and sometimes not.
ps_pledge is 64 bits so it's not possible to unset bits in an atomic
manner on all architectures. Even if we're only removing bits and there
is probably no way to see a completely garbage value, it makes sense to
just protect ps_pledge (and ps_execpledge) in the usual manner so that
we can unlock the syscall. The diff below protects the fields using
ps_mtx even though I initially used a dedicated ps_pledge_mtx.
unveil_destroy() needs to be moved after the critical section.
regress/sys/kern/pledge looks happy with this. The sys/syscall_mi.h
change can be committed in a separate step.
Input and oks welcome.
Index: arch/amd64/amd64/vmm.c
===================================================================
RCS file: /home/cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.315
diff -u -p -r1.315 vmm.c
--- arch/amd64/amd64/vmm.c 27 Jun 2022 15:12:14 -0000 1.315
+++ arch/amd64/amd64/vmm.c 28 Jun 2022 13:54:25 -0000
@@ -713,7 +713,7 @@ pledge_ioctl_vmm(struct proc *p, long co
case VMM_IOC_CREATE:
case VMM_IOC_INFO:
/* The "parent" process in vmd forks and manages VMs */
- if (p->p_p->ps_pledge & PLEDGE_PROC)
+ if (pledge_get(p->p_p) & PLEDGE_PROC)
return (0);
break;
case VMM_IOC_TERM:
@@ -1312,7 +1312,7 @@ vm_find(uint32_t id, struct vm **res)
* The managing vmm parent process can lookup all
* all VMs and is indicated by PLEDGE_PROC.
*/
- if (((p->p_p->ps_pledge &
+ if (((pledge_get(p->p_p) &
(PLEDGE_VMM | PLEDGE_PROC)) == PLEDGE_VMM) &&
(vm->vm_creator_pid != p->p_p->ps_pid))
return (pledge_fail(p, EPERM, PLEDGE_VMM));
Index: kern/init_sysent.c
===================================================================
RCS file: /home/cvs/src/sys/kern/init_sysent.c,v
retrieving revision 1.238
diff -u -p -r1.238 init_sysent.c
--- kern/init_sysent.c 27 Jun 2022 14:26:05 -0000 1.238
+++ kern/init_sysent.c 28 Jun 2022 15:18:25 -0000
@@ -1,10 +1,10 @@
-/* $OpenBSD: init_sysent.c,v 1.238 2022/06/27 14:26:05 cheloha Exp $
*/
+/* $OpenBSD$ */
/*
* 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 <sys/param.h>
@@ -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 */
Index: kern/kern_event.c
===================================================================
RCS file: /home/cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.191
diff -u -p -r1.191 kern_event.c
--- kern/kern_event.c 27 Jun 2022 13:35:21 -0000 1.191
+++ kern/kern_event.c 28 Jun 2022 13:55:18 -0000
@@ -331,7 +331,7 @@ filt_procattach(struct knote *kn)
int s;
if ((curproc->p_p->ps_flags & PS_PLEDGE) &&
- (curproc->p_p->ps_pledge & PLEDGE_PROC) == 0)
+ (pledge_get(curproc->p_p) & PLEDGE_PROC) == 0)
return pledge_fail(curproc, EPERM, PLEDGE_PROC);
if (kn->kn_id > PID_MAX)
Index: kern/kern_pledge.c
===================================================================
RCS file: /home/cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.282
diff -u -p -r1.282 kern_pledge.c
--- kern/kern_pledge.c 26 Jun 2022 06:11:49 -0000 1.282
+++ kern/kern_pledge.c 28 Jun 2022 15:21:46 -0000
@@ -21,6 +21,7 @@
#include <sys/mount.h>
#include <sys/proc.h>
+#include <sys/mutex.h>
#include <sys/fcntl.h>
#include <sys/file.h>
#include <sys/filedesc.h>
@@ -465,13 +466,26 @@ sys_pledge(struct proc *p, void *v, regi
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,43 +494,55 @@ sys_pledge(struct proc *p, void *v, regi
/* 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);
}
int
pledge_syscall(struct proc *p, int code, uint64_t *tval)
{
+ uint64_t pledge;
+
p->p_pledge_syscall = code;
*tval = 0;
@@ -526,7 +552,8 @@ pledge_syscall(struct proc *p, int code,
if (pledge_syscalls[code] == PLEDGE_ALWAYS)
return (0);
- if (p->p_p->ps_pledge & pledge_syscalls[code])
+ pledge = pledge_get(p->p_p);;
+ if (pledge & pledge_syscalls[code])
return (0);
*tval = pledge_syscalls[code];
@@ -549,7 +576,7 @@ pledge_fail(struct proc *p, int error, u
if (KTRPOINT(p, KTR_PLEDGE))
ktrpledge(p, error, code, p->p_pledge_syscall);
#endif
- if (p->p_p->ps_pledge & PLEDGE_ERROR)
+ if (pledge_get(p->p_p) & PLEDGE_ERROR)
return (ENOSYS);
KERNEL_LOCK();
@@ -583,7 +610,7 @@ pledge_namei(struct proc *p, struct name
if ((p->p_p->ps_flags & PS_PLEDGE) == 0 ||
(p->p_p->ps_flags & PS_COREDUMP))
return (0);
- pledge = p->p_p->ps_pledge;
+ pledge = pledge_get(p->p_p);
if (ni->ni_pledge == 0)
panic("pledge_namei: ni_pledge");
@@ -704,11 +731,12 @@ pledge_namei(struct proc *p, struct name
* 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);
+ pledge |= PLEDGE_YPACTIVE;
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
return (0);
}
@@ -770,7 +798,7 @@ pledge_recvfd(struct proc *p, struct fil
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & PLEDGE_RECVFD) == 0)
+ if ((pledge_get(p->p_p) & PLEDGE_RECVFD) == 0)
return pledge_fail(p, EPERM, PLEDGE_RECVFD);
switch (fp->f_type) {
@@ -798,7 +826,7 @@ pledge_sendfd(struct proc *p, struct fil
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & PLEDGE_SENDFD) == 0)
+ if ((pledge_get(p->p_p) & PLEDGE_SENDFD) == 0)
return pledge_fail(p, EPERM, PLEDGE_SENDFD);
switch (fp->f_type) {
@@ -826,7 +854,7 @@ pledge_sysctl(struct proc *p, int miblen
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- pledge = p->p_p->ps_pledge;
+ pledge = pledge_get(p->p_p);
if (new)
return pledge_fail(p, EFAULT, 0);
@@ -1023,7 +1051,7 @@ pledge_chown(struct proc *p, uid_t uid,
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if (p->p_p->ps_pledge & PLEDGE_CHOWNUID)
+ if (pledge_get(p->p_p) & PLEDGE_CHOWNUID)
return (0);
if (uid != -1 && uid != p->p_ucred->cr_uid)
@@ -1041,7 +1069,7 @@ pledge_adjtime(struct proc *p, const voi
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & PLEDGE_SETTIME))
+ if ((pledge_get(p->p_p) & PLEDGE_SETTIME))
return (0);
if (delta)
return (EPERM);
@@ -1054,7 +1082,7 @@ pledge_sendit(struct proc *p, const void
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & (PLEDGE_INET | PLEDGE_UNIX | PLEDGE_DNS |
PLEDGE_YPACTIVE)))
+ if ((pledge_get(p->p_p) & (PLEDGE_INET | PLEDGE_UNIX | PLEDGE_DNS |
PLEDGE_YPACTIVE)))
return (0); /* may use address */
if (to == NULL)
return (0); /* behaves just like write */
@@ -1070,7 +1098,7 @@ pledge_ioctl(struct proc *p, long com, s
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- pledge = p->p_p->ps_pledge;
+ pledge = pledge_get(p->p_p);
/*
* The ioctl's which are always allowed.
@@ -1365,7 +1393,7 @@ pledge_sockopt(struct proc *p, int set,
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- pledge = p->p_p->ps_pledge;
+ pledge = pledge_get(p->p_p);
/* Always allow these, which are too common to reject */
switch (level) {
@@ -1515,7 +1543,7 @@ pledge_socket(struct proc *p, int domain
if (!ISSET(p->p_p->ps_flags, PS_PLEDGE))
return 0;
- pledge = p->p_p->ps_pledge;
+ pledge = pledge_get(p->p_p);
if (ISSET(state, SS_DNS)) {
if (ISSET(pledge, PLEDGE_DNS))
@@ -1548,7 +1576,7 @@ pledge_flock(struct proc *p)
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & PLEDGE_FLOCK))
+ if ((pledge_get(p->p_p) & PLEDGE_FLOCK))
return (0);
return (pledge_fail(p, EPERM, PLEDGE_FLOCK));
}
@@ -1585,7 +1613,7 @@ pledge_fcntl(struct proc *p, int cmd)
{
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return (0);
- if ((p->p_p->ps_pledge & PLEDGE_PROC) == 0 && cmd == F_SETOWN)
+ if ((pledge_get(p->p_p) & PLEDGE_PROC) == 0 && cmd == F_SETOWN)
return pledge_fail(p, EPERM, PLEDGE_PROC);
return (0);
}
@@ -1595,7 +1623,7 @@ pledge_kill(struct proc *p, pid_t pid)
{
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
return 0;
- if (p->p_p->ps_pledge & PLEDGE_PROC)
+ if (pledge_get(p->p_p) & PLEDGE_PROC)
return 0;
if (pid == 0 || pid == p->p_p->ps_pid)
return 0;
@@ -1610,7 +1638,7 @@ pledge_protexec(struct proc *p, int prot
/* Before kbind(2) call, ld.so and crt may create EXEC mappings */
if (p->p_p->ps_kbind_addr == 0 && p->p_p->ps_kbind_cookie == 0)
return 0;
- if (!(p->p_p->ps_pledge & PLEDGE_PROTEXEC) && (prot & PROT_EXEC))
+ if (!(pledge_get(p->p_p) & PLEDGE_PROTEXEC) && (prot & PROT_EXEC))
return pledge_fail(p, EPERM, PLEDGE_PROTEXEC);
return 0;
}
Index: kern/syscalls.c
===================================================================
RCS file: /home/cvs/src/sys/kern/syscalls.c,v
retrieving revision 1.236
diff -u -p -r1.236 syscalls.c
--- kern/syscalls.c 16 May 2022 07:38:10 -0000 1.236
+++ kern/syscalls.c 28 Jun 2022 15:18:25 -0000
@@ -1,10 +1,10 @@
-/* $OpenBSD: syscalls.c,v 1.236 2022/05/16 07:38:10 mvs Exp $ */
+/* $OpenBSD$ */
/*
* 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[] = {
Index: kern/syscalls.master
===================================================================
RCS file: /home/cvs/src/sys/kern/syscalls.master,v
retrieving revision 1.225
diff -u -p -r1.225 syscalls.master
--- kern/syscalls.master 27 Jun 2022 14:26:05 -0000 1.225
+++ kern/syscalls.master 28 Jun 2022 12:47:19 -0000
@@ -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, \
Index: sys/proc.h
===================================================================
RCS file: /home/cvs/src/sys/sys/proc.h,v
retrieving revision 1.331
diff -u -p -r1.331 proc.h
--- sys/proc.h 27 Jun 2022 14:26:05 -0000 1.331
+++ sys/proc.h 28 Jun 2022 15:23:12 -0000
@@ -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] */
@@ -566,6 +566,19 @@ refreshcreds(struct proc *p)
if (pr->ps_ucred != p->p_ucred)
dorefreshcreds(pr, p);
}
+
+static inline uint64_t
+pledge_get(struct process *pr)
+{
+ uint64_t promises;
+
+ mtx_enter(&pr->ps_mtx);
+ promises = pr->ps_pledge;
+ mtx_leave(&pr->ps_mtx);
+
+ return promises;
+}
+
enum single_thread_mode {
SINGLE_SUSPEND, /* other threads to stop wherever they are */
Index: sys/syscall.h
===================================================================
RCS file: /home/cvs/src/sys/sys/syscall.h,v
retrieving revision 1.235
diff -u -p -r1.235 syscall.h
--- sys/syscall.h 27 Jun 2022 14:26:06 -0000 1.235
+++ sys/syscall.h 28 Jun 2022 15:18:25 -0000
@@ -1,10 +1,10 @@
-/* $OpenBSD: syscall.h,v 1.235 2022/06/27 14:26:06 cheloha Exp $ */
+/* $OpenBSD$ */
/*
* 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" "..." */
Index: sys/syscall_mi.h
===================================================================
RCS file: /home/cvs/src/sys/sys/syscall_mi.h,v
retrieving revision 1.25
diff -u -p -r1.25 syscall_mi.h
--- sys/syscall_mi.h 21 Jan 2020 16:16:23 -0000 1.25
+++ sys/syscall_mi.h 28 Jun 2022 12:43:07 -0000
@@ -89,16 +89,15 @@ mi_syscall(struct proc *p, register_t co
uvm_map_inentry_pc, p->p_vmspace->vm_map.wserial))
return (EPERM);
- if (lock)
- KERNEL_LOCK();
pledged = (p->p_p->ps_flags & PS_PLEDGE);
if (pledged && (error = pledge_syscall(p, code, &tval))) {
- if (!lock)
- KERNEL_LOCK();
+ KERNEL_LOCK();
error = pledge_fail(p, error, tval);
KERNEL_UNLOCK();
return (error);
}
+ if (lock)
+ KERNEL_LOCK();
error = (*callp->sy_call)(p, argp, retval);
if (lock)
KERNEL_UNLOCK();
Index: sys/syscallargs.h
===================================================================
RCS file: /home/cvs/src/sys/sys/syscallargs.h,v
retrieving revision 1.238
diff -u -p -r1.238 syscallargs.h
--- sys/syscallargs.h 27 Jun 2022 14:26:06 -0000 1.238
+++ sys/syscallargs.h 28 Jun 2022 15:18:25 -0000
@@ -1,10 +1,10 @@
-/* $OpenBSD: syscallargs.h,v 1.238 2022/06/27 14:26:06 cheloha Exp $
*/
+/* $OpenBSD$ */
/*
* 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
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
