Changeset View
Standalone View
sys/kern/kern_descrip.c
Show All 40 Lines | |||||
#include "opt_capsicum.h" | #include "opt_capsicum.h" | ||||
#include "opt_ddb.h" | #include "opt_ddb.h" | ||||
#include "opt_ktrace.h" | #include "opt_ktrace.h" | ||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/systm.h> | #include <sys/systm.h> | ||||
#include <sys/caprights.h> | |||||
markj: Do we still need this include? | |||||
#include <sys/capsicum.h> | #include <sys/capsicum.h> | ||||
#include <sys/conf.h> | #include <sys/conf.h> | ||||
#include <sys/fcntl.h> | #include <sys/fcntl.h> | ||||
#include <sys/file.h> | #include <sys/file.h> | ||||
#include <sys/filedesc.h> | #include <sys/filedesc.h> | ||||
#include <sys/filio.h> | #include <sys/filio.h> | ||||
#include <sys/jail.h> | #include <sys/jail.h> | ||||
#include <sys/kernel.h> | #include <sys/kernel.h> | ||||
▲ Show 20 Lines • Show All 421 Lines • ▼ Show 20 Lines | if (cmd == F_OGETLK) { | ||||
error = copyout(&fl, (void *)(intptr_t)arg, sizeof(fl)); | error = copyout(&fl, (void *)(intptr_t)arg, sizeof(fl)); | ||||
} | } | ||||
return (error); | return (error); | ||||
} | } | ||||
int | int | ||||
kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg) | kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg) | ||||
{ | { | ||||
cap_rights_t rights; | |||||
struct filedesc *fdp; | struct filedesc *fdp; | ||||
struct flock *flp; | struct flock *flp; | ||||
struct file *fp, *fp2; | struct file *fp, *fp2; | ||||
struct filedescent *fde; | struct filedescent *fde; | ||||
struct proc *p; | struct proc *p; | ||||
struct vnode *vp; | struct vnode *vp; | ||||
int error, flg, tmp; | int error, flg, seals, tmp; | ||||
uint64_t bsize; | uint64_t bsize; | ||||
off_t foffset; | off_t foffset; | ||||
error = 0; | error = 0; | ||||
flg = F_POSIX; | flg = F_POSIX; | ||||
p = td->td_proc; | p = td->td_proc; | ||||
fdp = p->p_fd; | fdp = p->p_fd; | ||||
▲ Show 20 Lines • Show All 247 Lines • ▼ Show 20 Lines | if (flp->l_whence == SEEK_CUR) { | ||||
fdrop(fp, td); | fdrop(fp, td); | ||||
break; | break; | ||||
} | } | ||||
flp->l_start += foffset; | flp->l_start += foffset; | ||||
} | } | ||||
vp = fp->f_vnode; | vp = fp->f_vnode; | ||||
error = VOP_ADVLOCK(vp, (caddr_t)p->p_leader, F_GETLK, flp, | error = VOP_ADVLOCK(vp, (caddr_t)p->p_leader, F_GETLK, flp, | ||||
F_POSIX); | F_POSIX); | ||||
fdrop(fp, td); | |||||
break; | |||||
case F_ADD_SEALS: | |||||
error = fget_unlocked(fdp, fd, &cap_no_rights, &fp, NULL); | |||||
if (error != 0) | |||||
break; | |||||
if ((fp->f_flag & (FWRITE | FSEALABLE)) != (FWRITE | FSEALABLE)) | |||||
error = EINVAL; | |||||
else if (fo_get_seals(fp, &seals) != 0) | |||||
error = EINVAL; | |||||
else if ((seals & F_SEAL_SEAL) != 0) | |||||
Done Inline ActionsThis is weird. IMO it should be checked (atomically) in fo_add_seals(). Wouldn't it allow a race where one thread already set F_SEAL_SEAL while other thread did not see it after fo_get_seals() but fo_add_seals() cannot proceed ? In general, what are the desired atomicity requirements for interaction between seals and in-flight ops ? E.g. I believe that the current arrangement allows for write(2) to verifiable process while other thread sets the seal on shmfd. Do we declare that as a user race, or do we want to ensure that no write can be in progress after set seal for write fcntl returned ? kib: This is weird. IMO it should be checked (atomically) in fo_add_seals().
Wouldn't it allow a… | |||||
Done Inline ActionsHmm... yeah, true- I'll whittle this down to fo_add_seals() alone and deal with F_SEAL_SEAL in fo_add_seals. Linux manpage only seems to guarantee that the seals are enforced once fcntl(..., F_ADD_SEALS) has successfully returned: "Once this call succeeds, the seals are enforced by the kernel immediately." -> I interpret this as deeming it a user race. I think most users of seals would tend towards creating the file, operating on it, then sealing it before passing it around. kevans: Hmm... yeah, true- I'll whittle this down to fo_add_seals() alone and deal with F_SEAL_SEAL in… | |||||
Done Inline ActionsI would be very skeptical to infer anything from linux manpage, esp. such fine details. If somebody could read the implementation, or ask authors of the API about the intent ... I understand that this might be too much to ask. kib: I would be very skeptical to infer anything from linux manpage, esp. such fine details. If… | |||||
Done Inline ActionsAs far as I understand, the intent is to avoid the need for userland to implement such complicated things as the Xorg server's busfault (see https://lists.x.org/archives/xorg-devel/2013-November/038598.html ). This is further described in the NOTES section of http://man7.org/linux/man-pages/man2/memfd_create.2.html . Also note the extra properties of F_SEAL_WRITE in http://man7.org/linux/man-pages/man2/fcntl.2.html : the sealing call fails if a writable MAP_SHARED mapping exists, and aborts any asynchronous I/O. As such, any conflicting modification visible after a successful F_ADD_SEALS or F_GET_SEALS might lead to a security hole. However, when adding seals, this still leaves the kernel the choice of waiting for the conflicting modification to end, synchronously cancelling it or denying the seal change. Atomicity on the F_SEAL_SEAL bit seems less important. jilles: As far as I understand, the intent is to avoid the need for userland to implement such… | |||||
error = EPERM; | |||||
if (error != 0) { | |||||
fdrop(fp, td); | |||||
break; | |||||
} | |||||
tmp = (arg & ~seals); | |||||
/* Nop; the rest isn't worth evaluating */ | |||||
if (tmp == 0) { | |||||
fdrop(fp, td); | |||||
break; | |||||
} | |||||
/* This shouldn't fail; if it does, we can't act on this seal */ | |||||
error = fo_add_seals(fp, arg); | |||||
if (error != 0) { | |||||
fdrop(fp, td); | |||||
break; | |||||
} | |||||
/* F_SEAL_WRITE translates to revoking CAP_WRITE */ | |||||
if ((tmp & F_SEAL_WRITE) != 0) { | |||||
rights = *cap_rights(fdp, fd); | |||||
cap_rights_clear(&rights, CAP_WRITE); | |||||
kern_cap_rights_limit(td, fd, &rights); | |||||
} | |||||
fdrop(fp, td); | |||||
break; | |||||
case F_GET_SEALS: | |||||
error = fget_unlocked(fdp, fd, &cap_no_rights, &fp, NULL); | |||||
if (error != 0) | |||||
break; | |||||
if ((fp->f_flag & FSEALABLE) != 0 && | |||||
fo_get_seals(fp, &seals) == 0) | |||||
td->td_retval[0] = seals; | |||||
else | |||||
error = EINVAL; | |||||
fdrop(fp, td); | fdrop(fp, td); | ||||
break; | break; | ||||
case F_RDAHEAD: | case F_RDAHEAD: | ||||
arg = arg ? 128 * 1024: 0; | arg = arg ? 128 * 1024: 0; | ||||
/* FALLTHROUGH */ | /* FALLTHROUGH */ | ||||
case F_READAHEAD: | case F_READAHEAD: | ||||
error = fget_unlocked(fdp, fd, &cap_no_rights, &fp, NULL); | error = fget_unlocked(fdp, fd, &cap_no_rights, &fp, NULL); | ||||
▲ Show 20 Lines • Show All 3,497 Lines • Show Last 20 Lines |
Do we still need this include?