From 215f41900c1e2aba2851cec2e69220a2194c2a6d Mon Sep 17 00:00:00 2001 From: deraadt Date: Mon, 15 Nov 2021 17:14:51 +0000 Subject: [PATCH] Copy p_p->ps_pledge into a local variable (called pledge) in every function which checks PLEDGE_* bits more than once. Some functions are called without locking, and this avoids misinterpreting bits which have some coupled behaviour. ok cheloha kettenis --- sys/kern/kern_pledge.c | 119 ++++++++++++++++++++++------------------- 1 file changed, 65 insertions(+), 54 deletions(-) diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c index 1fa85ccf603..2b9a1a0abc2 100644 --- a/sys/kern/kern_pledge.c +++ b/sys/kern/kern_pledge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_pledge.c,v 1.275 2021/06/29 01:46:35 jsg Exp $ */ +/* $OpenBSD: kern_pledge.c,v 1.276 2021/11/15 17:14:51 deraadt Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -564,11 +564,13 @@ int pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) { char path[PATH_MAX]; + uint64_t pledge; int error; if ((p->p_p->ps_flags & PS_PLEDGE) == 0 || (p->p_p->ps_flags & PS_COREDUMP)) return (0); + pledge = p->p_p->ps_pledge; if (ni->ni_pledge == 0) panic("pledge_namei: ni_pledge"); @@ -580,7 +582,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* Doing a permitted execve() */ if ((ni->ni_pledge & PLEDGE_EXEC) && - (p->p_p->ps_pledge & PLEDGE_EXEC)) + (pledge & PLEDGE_EXEC)) return (0); error = canonpath(origpath, path, sizeof(path)); @@ -588,7 +590,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) return (error); /* Detect what looks like a mkstemp(3) family operation */ - if ((p->p_p->ps_pledge & PLEDGE_TMPPATH) && + if ((pledge & PLEDGE_TMPPATH) && (p->p_pledge_syscall == SYS_open) && (ni->ni_pledge & PLEDGE_CPATH) && strncmp(path, "/tmp/", sizeof("/tmp/") - 1) == 0) { @@ -599,7 +601,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* Allow unlinking of a mkstemp(3) file... * Good opportunity for strict checks here. */ - if ((p->p_p->ps_pledge & PLEDGE_TMPPATH) && + if ((pledge & PLEDGE_TMPPATH) && (p->p_pledge_syscall == SYS_unlink) && strncmp(path, "/tmp/", sizeof("/tmp/") - 1) == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; @@ -619,7 +621,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* when avoiding YP mode, getpw* functions touch this */ if (ni->ni_pledge == PLEDGE_RPATH && strcmp(path, "/var/run/ypbind.lock") == 0) { - if (p->p_p->ps_pledge & PLEDGE_GETPW) { + if (pledge & PLEDGE_GETPW) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); } else @@ -635,7 +637,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) } /* readpassphrase(3), getpass(3) */ - if ((p->p_p->ps_pledge & PLEDGE_TTY) && + if ((pledge & PLEDGE_TTY) && (ni->ni_pledge & ~(PLEDGE_RPATH | PLEDGE_WPATH)) == 0 && strcmp(path, "/dev/tty") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; @@ -644,7 +646,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* getpw* and friends need a few files */ if ((ni->ni_pledge == PLEDGE_RPATH) && - (p->p_p->ps_pledge & PLEDGE_GETPW)) { + (pledge & PLEDGE_GETPW)) { if (strcmp(path, "/etc/spwd.db") == 0) return (EPERM); /* don't call pledge_fail */ if (strcmp(path, "/etc/pwd.db") == 0) { @@ -663,7 +665,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) /* DNS needs /etc/{resolv.conf,hosts,services,protocols}. */ if ((ni->ni_pledge == PLEDGE_RPATH) && - (p->p_p->ps_pledge & PLEDGE_DNS)) { + (pledge & PLEDGE_DNS)) { if (strcmp(path, "/etc/resolv.conf") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); @@ -683,15 +685,15 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) } if ((ni->ni_pledge == PLEDGE_RPATH) && - (p->p_p->ps_pledge & PLEDGE_GETPW)) { + (pledge & PLEDGE_GETPW)) { if (strcmp(path, "/var/run/ypbind.lock") == 0) { /* * XXX * The current hack for YP support in "getpw" * is to enable some "inet" features until - * next pledge call. This is not considered - * worse than pre-pledge, but is a work in - * progress, needing a clever design. + * next pledge call. Setting a bit in ps_pledge + * is not safe with respect to multiple threads, + * a very different approach is needed. */ p->p_p->ps_pledge |= PLEDGE_YPACTIVE; ni->ni_cnd.cn_flags |= BYPASSUNVEIL; @@ -721,7 +723,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) case SYS_stat: /* DNS needs /etc/resolv.conf. */ if ((ni->ni_pledge == PLEDGE_RPATH) && - (p->p_p->ps_pledge & PLEDGE_DNS) && + (pledge & PLEDGE_DNS) && strcmp(path, "/etc/resolv.conf") == 0) { ni->ni_cnd.cn_flags |= BYPASSUNVEIL; return (0); @@ -733,8 +735,8 @@ pledge_namei(struct proc *p, struct nameidata *ni, char *origpath) * Ensure each flag of ni_pledge has counterpart allowing it in * ps_pledge. */ - if (ni->ni_pledge & ~p->p_p->ps_pledge) - return (pledge_fail(p, EPERM, (ni->ni_pledge & ~p->p_p->ps_pledge))); + if (ni->ni_pledge & ~pledge) + return (pledge_fail(p, EPERM, (ni->ni_pledge & ~pledge))); /* continue, and check unveil if present */ return (0); @@ -799,16 +801,18 @@ int pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) { char buf[80]; + uint64_t pledge; int i; if ((p->p_p->ps_flags & PS_PLEDGE) == 0) return (0); + pledge = p->p_p->ps_pledge; if (new) return pledge_fail(p, EFAULT, 0); /* routing table observation */ - if ((p->p_p->ps_pledge & PLEDGE_ROUTE)) { + if ((pledge & PLEDGE_ROUTE)) { if ((miblen == 6 || miblen == 7) && mib[0] == CTL_NET && mib[1] == PF_ROUTE && mib[2] == 0 && @@ -830,14 +834,14 @@ pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) return (0); } - if ((p->p_p->ps_pledge & PLEDGE_WROUTE)) { + if ((pledge & PLEDGE_WROUTE)) { if (miblen == 4 && mib[0] == CTL_NET && mib[1] == PF_INET6 && mib[2] == IPPROTO_IPV6 && mib[3] == IPV6CTL_SOIIKEY) return (0); } - if (p->p_p->ps_pledge & (PLEDGE_PS | PLEDGE_VMINFO)) { + if (pledge & (PLEDGE_PS | PLEDGE_VMINFO)) { if (miblen == 2 && /* kern.fscale */ mib[0] == CTL_KERN && mib[1] == KERN_FSCALE) return (0); @@ -858,7 +862,7 @@ pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) return (0); } - if ((p->p_p->ps_pledge & PLEDGE_PS)) { + if ((pledge & PLEDGE_PS)) { if (miblen == 4 && /* kern.procargs.* */ mib[0] == CTL_KERN && mib[1] == KERN_PROC_ARGS && (mib[3] == KERN_PROC_ARGV || mib[3] == KERN_PROC_ENV)) @@ -877,7 +881,7 @@ pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) return (0); } - if ((p->p_p->ps_pledge & PLEDGE_VMINFO)) { + if ((pledge & PLEDGE_VMINFO)) { if (miblen == 2 && /* vm.uvmexp */ mib[0] == CTL_VM && mib[1] == VM_UVMEXP) return (0); @@ -887,13 +891,13 @@ pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) return (0); } - if ((p->p_p->ps_pledge & (PLEDGE_INET | PLEDGE_UNIX))) { + if ((pledge & (PLEDGE_INET | PLEDGE_UNIX))) { if (miblen == 2 && /* kern.somaxconn */ mib[0] == CTL_KERN && mib[1] == KERN_SOMAXCONN) return (0); } - if ((p->p_p->ps_pledge & (PLEDGE_ROUTE | PLEDGE_INET | PLEDGE_DNS))) { + if ((pledge & (PLEDGE_ROUTE | PLEDGE_INET | PLEDGE_DNS))) { if (miblen == 6 && /* getifaddrs() */ mib[0] == CTL_NET && mib[1] == PF_ROUTE && mib[2] == 0 && @@ -902,7 +906,7 @@ pledge_sysctl(struct proc *p, int miblen, int *mib, void *new) return (0); } - if ((p->p_p->ps_pledge & PLEDGE_DISKLABEL)) { + if ((pledge & PLEDGE_DISKLABEL)) { if (miblen == 2 && /* kern.rawpartition */ mib[0] == CTL_KERN && mib[1] == KERN_RAWPARTITION) @@ -1035,10 +1039,11 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) { struct vnode *vp = NULL; int error = EPERM; - uint64_t pl = p->p_p->ps_pledge; + uint64_t pledge; if ((p->p_p->ps_flags & PS_PLEDGE) == 0) return (0); + pledge = p->p_p->ps_pledge; /* * The ioctl's which are always allowed. @@ -1058,7 +1063,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) return (ENOTTY); } - if ((pl & PLEDGE_INET)) { + if ((pledge & PLEDGE_INET)) { switch (com) { case SIOCATMARK: case SIOCGIFGROUP: @@ -1069,7 +1074,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #if NBPFILTER > 0 - if ((pl & PLEDGE_BPF)) { + if ((pledge & PLEDGE_BPF)) { switch (com) { case BIOCGSTATS: /* bpf: tcpdump privsep on ^C */ if (fp->f_type == DTYPE_VNODE && @@ -1082,7 +1087,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #endif /* NBPFILTER > 0 */ - if ((pl & PLEDGE_TAPE)) { + if ((pledge & PLEDGE_TAPE)) { switch (com) { case MTIOCGET: case MTIOCTOP: @@ -1099,7 +1104,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #if NDRM > 0 - if ((pl & PLEDGE_DRM)) { + if ((pledge & PLEDGE_DRM)) { if ((fp->f_type == DTYPE_VNODE) && (vp->v_type == VCHR) && (cdevsw[major(vp->v_rdev)].d_open == drmopen)) { @@ -1111,7 +1116,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) #endif /* NDRM > 0 */ #if NAUDIO > 0 - if ((pl & PLEDGE_AUDIO)) { + if ((pledge & PLEDGE_AUDIO)) { switch (com) { case AUDIO_GETPOS: case AUDIO_GETPAR: @@ -1129,7 +1134,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #endif /* NAUDIO > 0 */ - if ((pl & PLEDGE_DISKLABEL)) { + if ((pledge & PLEDGE_DISKLABEL)) { switch (com) { case DIOCGDINFO: case DIOCGPDINFO: @@ -1156,7 +1161,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #if NVIDEO > 0 - if ((pl & PLEDGE_VIDEO)) { + if ((pledge & PLEDGE_VIDEO)) { switch (com) { case VIDIOC_QUERYCAP: case VIDIOC_TRY_FMT: @@ -1198,7 +1203,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) #endif #if NPF > 0 - if ((pl & PLEDGE_PF)) { + if ((pledge & PLEDGE_PF)) { switch (com) { case DIOCADDRULE: case DIOCGETSTATUS: @@ -1221,13 +1226,13 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #endif - if ((pl & PLEDGE_TTY)) { + if ((pledge & PLEDGE_TTY)) { switch (com) { #if NPTY > 0 case PTMGET: - if ((pl & PLEDGE_RPATH) == 0) + if ((pledge & PLEDGE_RPATH) == 0) break; - if ((pl & PLEDGE_WPATH) == 0) + if ((pledge & PLEDGE_WPATH) == 0) break; if (fp->f_type != DTYPE_VNODE || vp->v_type != VCHR) break; @@ -1235,16 +1240,16 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) break; return (0); case TIOCUCNTL: /* vmd */ - if ((pl & PLEDGE_RPATH) == 0) + if ((pledge & PLEDGE_RPATH) == 0) break; - if ((pl & PLEDGE_WPATH) == 0) + if ((pledge & PLEDGE_WPATH) == 0) break; if (cdevsw[major(vp->v_rdev)].d_open != ptcopen) break; return (0); #endif /* NPTY > 0 */ case TIOCSPGRP: - if ((pl & PLEDGE_PROC) == 0) + if ((pledge & PLEDGE_PROC) == 0) break; /* FALLTHROUGH */ case TIOCFLUSH: /* getty, telnet */ @@ -1273,7 +1278,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } } - if ((pl & PLEDGE_ROUTE)) { + if ((pledge & PLEDGE_ROUTE)) { switch (com) { case SIOCGIFADDR: case SIOCGIFAFLAG_IN6: @@ -1295,7 +1300,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } } - if ((pl & PLEDGE_WROUTE)) { + if ((pledge & PLEDGE_WROUTE)) { switch (com) { case SIOCAIFADDR: case SIOCDIFADDR: @@ -1312,7 +1317,7 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) } #if NVMM > 0 - if ((pl & PLEDGE_VMM)) { + if ((pledge & PLEDGE_VMM)) { if ((fp->f_type == DTYPE_VNODE) && (vp->v_type == VCHR) && (cdevsw[major(vp->v_rdev)].d_open == vmmopen)) { @@ -1329,8 +1334,11 @@ pledge_ioctl(struct proc *p, long com, struct file *fp) int pledge_sockopt(struct proc *p, int set, int level, int optname) { + uint64_t pledge; + if ((p->p_p->ps_flags & PS_PLEDGE) == 0) return (0); + pledge = p->p_p->ps_pledge; /* Always allow these, which are too common to reject */ switch (level) { @@ -1343,7 +1351,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) break; } - if ((p->p_p->ps_pledge & PLEDGE_WROUTE)) { + if ((pledge & PLEDGE_WROUTE)) { switch (level) { case SOL_SOCKET: switch (optname) { @@ -1353,7 +1361,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) } } - if ((p->p_p->ps_pledge & (PLEDGE_INET|PLEDGE_UNIX|PLEDGE_DNS|PLEDGE_YPACTIVE)) == 0) + if ((pledge & (PLEDGE_INET|PLEDGE_UNIX|PLEDGE_DNS|PLEDGE_YPACTIVE)) == 0) return pledge_fail(p, EPERM, PLEDGE_INET); /* In use by some service libraries */ switch (level) { @@ -1366,7 +1374,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) } /* DNS resolver may do these requests */ - if ((p->p_p->ps_pledge & PLEDGE_DNS)) { + if ((pledge & PLEDGE_DNS)) { switch (level) { case IPPROTO_IPV6: switch (optname) { @@ -1378,7 +1386,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) } /* YP may do these requests */ - if (p->p_p->ps_pledge & PLEDGE_YPACTIVE) { + if (pledge & PLEDGE_YPACTIVE) { switch (level) { case IPPROTO_IP: switch (optname) { @@ -1396,7 +1404,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) } } - if ((p->p_p->ps_pledge & (PLEDGE_INET|PLEDGE_UNIX)) == 0) + if ((pledge & (PLEDGE_INET|PLEDGE_UNIX)) == 0) return pledge_fail(p, EPERM, PLEDGE_INET); switch (level) { case SOL_SOCKET: @@ -1407,7 +1415,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) return (0); } - if ((p->p_p->ps_pledge & PLEDGE_INET) == 0) + if ((pledge & PLEDGE_INET) == 0) return pledge_fail(p, EPERM, PLEDGE_INET); switch (level) { case IPPROTO_TCP: @@ -1439,7 +1447,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) case IP_MULTICAST_LOOP: case IP_ADD_MEMBERSHIP: case IP_DROP_MEMBERSHIP: - if (p->p_p->ps_pledge & PLEDGE_MCAST) + if (pledge & PLEDGE_MCAST) return (0); break; } @@ -1462,7 +1470,7 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) case IPV6_MULTICAST_LOOP: case IPV6_JOIN_GROUP: case IPV6_LEAVE_GROUP: - if (p->p_p->ps_pledge & PLEDGE_MCAST) + if (pledge & PLEDGE_MCAST) return (0); break; } @@ -1476,11 +1484,14 @@ pledge_sockopt(struct proc *p, int set, int level, int optname) int pledge_socket(struct proc *p, int domain, unsigned int state) { - if (! ISSET(p->p_p->ps_flags, PS_PLEDGE)) + uint64_t pledge; + + if (!ISSET(p->p_p->ps_flags, PS_PLEDGE)) return 0; + pledge = p->p_p->ps_pledge; if (ISSET(state, SS_DNS)) { - if (ISSET(p->p_p->ps_pledge, PLEDGE_DNS)) + if (ISSET(pledge, PLEDGE_DNS)) return 0; return pledge_fail(p, EPERM, PLEDGE_DNS); } @@ -1490,13 +1501,13 @@ pledge_socket(struct proc *p, int domain, unsigned int state) return (0); case AF_INET: case AF_INET6: - if (ISSET(p->p_p->ps_pledge, PLEDGE_INET) || - ISSET(p->p_p->ps_pledge, PLEDGE_YPACTIVE)) + if (ISSET(pledge, PLEDGE_INET) || + ISSET(pledge, PLEDGE_YPACTIVE)) return 0; return pledge_fail(p, EPERM, PLEDGE_INET); case AF_UNIX: - if (ISSET(p->p_p->ps_pledge, PLEDGE_UNIX)) + if (ISSET(pledge, PLEDGE_UNIX)) return 0; return pledge_fail(p, EPERM, PLEDGE_UNIX); } -- 2.20.1