Changeset View
Standalone View
sys/kern/vfs_lookup.c
Show First 20 Lines • Show All 176 Lines • ▼ Show 20 Lines | |||||
#ifdef CAPABILITY_MODE | #ifdef CAPABILITY_MODE | ||||
/* | /* | ||||
* In capability mode, lookups must be "strictly relative" (i.e. | * In capability mode, lookups must be "strictly relative" (i.e. | ||||
* not an absolute path, and not containing '..' components) to | * not an absolute path, and not containing '..' components) to | ||||
* a real file descriptor, not the pseudo-descriptor AT_FDCWD. | * a real file descriptor, not the pseudo-descriptor AT_FDCWD. | ||||
*/ | */ | ||||
if (error == 0 && IN_CAPABILITY_MODE(td) && | if (error == 0 && IN_CAPABILITY_MODE(td) && | ||||
(cnp->cn_flags & NOCAPCHECK) == 0) { | (cnp->cn_flags & NOCAPCHECK) == 0) { | ||||
ndp->ni_strictrelative = 1; | if (ndp->ni_nonrelativeerrno != EPERM) | ||||
ndp->ni_nonrelativeerrno = ENOTCAPABLE; | |||||
if (ndp->ni_dirfd == AT_FDCWD) { | if (ndp->ni_dirfd == AT_FDCWD) { | ||||
mjg: The caller is free to specify any error they want, yet this checks specifically for EPERM. I… | |||||
Done Inline ActionsWe actually want to honour EPERM above ENOTCAPABLE, since the user application code can always set the O_BENEATH flag and might expect EPERM when there is a failure. In all other cases, we preserve the existing ABI by returning ENOTCAPABLE. jonathan: We actually want to honour `EPERM` above `ENOTCAPABLE`, since the user application code can… | |||||
Done Inline ActionsBut proposed change would honour EPERM just fine. Further, when one day other caller will want to return EACCESS, it will just work, while with current patch whatever it sets will be converted to ENOTCAPABLE. That's only a nit though. mjg: But proposed change would honour EPERM just fine. Further, when one day other caller will want… | |||||
Not Done Inline ActionsThat's a fair point. Another development (not visible here on the issue tracker) is that we're considering just creating a brand new errno for this kind of failure. That would provide more information to the caller about the true nature of the error. It would also allow us to stay compatible between the FreeBSD and Linux implementations even though Linux doesn't have ENOTCAPABLE, and we could #define this value to ENOTCAPABLE on FreeBSD 10 to maintain ABI compatibility. jonathan: That's a fair point. Another development (not visible here on the issue tracker) is that we're… | |||||
#ifdef KTRACE | #ifdef KTRACE | ||||
if (KTRPOINT(td, KTR_CAPFAIL)) | if (KTRPOINT(td, KTR_CAPFAIL)) | ||||
ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ||||
#endif | #endif | ||||
error = ECAPMODE; | error = ECAPMODE; | ||||
} | } | ||||
} | } | ||||
#endif | #endif | ||||
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines | if (ndp->ni_startdir != NULL) { | ||||
AUDIT_ARG_ATFD2(ndp->ni_dirfd); | AUDIT_ARG_ATFD2(ndp->ni_dirfd); | ||||
error = fgetvp_rights(td, ndp->ni_dirfd, | error = fgetvp_rights(td, ndp->ni_dirfd, | ||||
&rights, &ndp->ni_filecaps, &dp); | &rights, &ndp->ni_filecaps, &dp); | ||||
#ifdef CAPABILITIES | #ifdef CAPABILITIES | ||||
/* | /* | ||||
* If file descriptor doesn't have all rights, | * If file descriptor doesn't have all rights, | ||||
* all lookups relative to it must also be | * all lookups relative to it must also be | ||||
* strictly relative. | * strictly relative. | ||||
* | |||||
* Don't overwrite a value of EPERM, however: | |||||
* we need to honour a user O_BENEATH request. | |||||
*/ | */ | ||||
CAP_ALL(&rights); | CAP_ALL(&rights); | ||||
if (!cap_rights_contains(&ndp->ni_filecaps.fc_rights, | if ((!cap_rights_contains(&ndp->ni_filecaps.fc_rights, | ||||
&rights) || | &rights) || | ||||
ndp->ni_filecaps.fc_fcntls != CAP_FCNTL_ALL || | ndp->ni_filecaps.fc_fcntls != CAP_FCNTL_ALL || | ||||
ndp->ni_filecaps.fc_nioctls != -1) { | ndp->ni_filecaps.fc_nioctls != -1) && | ||||
ndp->ni_strictrelative = 1; | ndp->ni_nonrelativeerrno != EPERM) | ||||
} | ndp->ni_nonrelativeerrno = ENOTCAPABLE; | ||||
#endif | #endif | ||||
} | } | ||||
if (error != 0 || dp != NULL) { | if (error != 0 || dp != NULL) { | ||||
FILEDESC_SUNLOCK(fdp); | FILEDESC_SUNLOCK(fdp); | ||||
if (error == 0 && dp->v_type != VDIR) { | if (error == 0 && dp->v_type != VDIR) { | ||||
vrele(dp); | vrele(dp); | ||||
error = ENOTDIR; | error = ENOTDIR; | ||||
} | } | ||||
Show All 15 Lines | #endif | ||||
for (;;) { | for (;;) { | ||||
/* | /* | ||||
* Check if root directory should replace current directory. | * Check if root directory should replace current directory. | ||||
* Done at start of translation and after symbolic link. | * Done at start of translation and after symbolic link. | ||||
*/ | */ | ||||
cnp->cn_nameptr = cnp->cn_pnbuf; | cnp->cn_nameptr = cnp->cn_pnbuf; | ||||
if (*(cnp->cn_nameptr) == '/') { | if (*(cnp->cn_nameptr) == '/') { | ||||
vrele(dp); | vrele(dp); | ||||
if (ndp->ni_strictrelative != 0) { | if (ndp->ni_nonrelativeerrno != 0) { | ||||
#ifdef KTRACE | #ifdef KTRACE | ||||
if (KTRPOINT(curthread, KTR_CAPFAIL)) | if (KTRPOINT(curthread, KTR_CAPFAIL)) | ||||
ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ||||
#endif | #endif | ||||
namei_cleanup_cnp(cnp); | namei_cleanup_cnp(cnp); | ||||
return (ENOTCAPABLE); | return (ndp->ni_nonrelativeerrno); | ||||
} | } | ||||
while (*(cnp->cn_nameptr) == '/') { | while (*(cnp->cn_nameptr) == '/') { | ||||
cnp->cn_nameptr++; | cnp->cn_nameptr++; | ||||
ndp->ni_pathlen--; | ndp->ni_pathlen--; | ||||
} | } | ||||
dp = ndp->ni_rootdir; | dp = ndp->ni_rootdir; | ||||
VREF(dp); | VREF(dp); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 318 Lines • ▼ Show 20 Lines | #endif | ||||
* 3. If this vnode is the root of a mounted | * 3. If this vnode is the root of a mounted | ||||
* filesystem, then replace it with the | * filesystem, then replace it with the | ||||
* vnode which was mounted on so we take the | * vnode which was mounted on so we take the | ||||
* .. in the other filesystem. | * .. in the other filesystem. | ||||
* 4. If the vnode is the top directory of | * 4. If the vnode is the top directory of | ||||
* the jail or chroot, don't let them out. | * the jail or chroot, don't let them out. | ||||
*/ | */ | ||||
if (cnp->cn_flags & ISDOTDOT) { | if (cnp->cn_flags & ISDOTDOT) { | ||||
if (ndp->ni_strictrelative != 0) { | if (ndp->ni_nonrelativeerrno != 0) { | ||||
#ifdef KTRACE | #ifdef KTRACE | ||||
if (KTRPOINT(curthread, KTR_CAPFAIL)) | if (KTRPOINT(curthread, KTR_CAPFAIL)) | ||||
ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); | ||||
rwatsonUnsubmitted Done Inline ActionsHmm. If we're limited by O_BENEATH, should we be using a different KTR trace function here? rwatson: Hmm. If we're limited by O_BENEATH, should we be using a different KTR trace function here? | |||||
jonathanAuthorUnsubmitted Done Inline ActionsHow would you feel about using KTR_NAMEI instead? We could use KTR_CAPFAIL for Capsicum-caused failures and KTR_NAMEI for O_BENEATH-induced failures, but that's adding a lot of complexity and it's not entirely clear to me how we should handle the "both" case... jonathan: How would you feel about using `KTR_NAMEI` instead? We could use `KTR_CAPFAIL` for Capsicum… | |||||
rwatsonUnsubmitted Done Inline ActionsHurum. Agreed it would make things more complex, but it probably is the right thing. How about we simply make the decision based on whether it's ENOTCAPABLE or EPERM? rwatson: Hurum. Agreed it would make things more complex, but it probably is the right thing. How about… | |||||
jonathanAuthorUnsubmitted Not Done Inline ActionsOk, I can try that and see how it looks. jonathan: Ok, I can try that and see how it looks. | |||||
#endif | #endif | ||||
Not Done Inline ActionsOk, @rwatson, how does this ktrace update look? jonathan: Ok, @rwatson, how does this ktrace update look? | |||||
error = ENOTCAPABLE; | error = ndp->ni_nonrelativeerrno; | ||||
goto bad; | goto bad; | ||||
} | } | ||||
if ((cnp->cn_flags & ISLASTCN) != 0 && | if ((cnp->cn_flags & ISLASTCN) != 0 && | ||||
(cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)) { | (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)) { | ||||
error = EINVAL; | error = EINVAL; | ||||
goto bad; | goto bad; | ||||
} | } | ||||
for (;;) { | for (;;) { | ||||
▲ Show 20 Lines • Show All 418 Lines • ▼ Show 20 Lines | |||||
{ | { | ||||
ndp->ni_cnd.cn_nameiop = op; | ndp->ni_cnd.cn_nameiop = op; | ||||
ndp->ni_cnd.cn_flags = flags; | ndp->ni_cnd.cn_flags = flags; | ||||
ndp->ni_segflg = segflg; | ndp->ni_segflg = segflg; | ||||
ndp->ni_dirp = namep; | ndp->ni_dirp = namep; | ||||
ndp->ni_dirfd = dirfd; | ndp->ni_dirfd = dirfd; | ||||
ndp->ni_startdir = startdir; | ndp->ni_startdir = startdir; | ||||
ndp->ni_strictrelative = 0; | ndp->ni_nonrelativeerrno = 0; | ||||
if (rightsp != NULL) | if (rightsp != NULL) | ||||
ndp->ni_rightsneeded = *rightsp; | ndp->ni_rightsneeded = *rightsp; | ||||
else | else | ||||
cap_rights_init(&ndp->ni_rightsneeded); | cap_rights_init(&ndp->ni_rightsneeded); | ||||
filecaps_init(&ndp->ni_filecaps); | filecaps_init(&ndp->ni_filecaps); | ||||
ndp->ni_cnd.cn_thread = td; | ndp->ni_cnd.cn_thread = td; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Lines | |||||
* alternate prefix will be used so long as the parent directory exists. | * alternate prefix will be used so long as the parent directory exists. | ||||
* This is used by the various compatiblity ABIs so that Linux binaries prefer | * This is used by the various compatiblity ABIs so that Linux binaries prefer | ||||
* files under /compat/linux for example. The chosen path (whether under | * files under /compat/linux for example. The chosen path (whether under | ||||
* the prefix or under /) is returned in a kernel malloc'd buffer pointed | * the prefix or under /) is returned in a kernel malloc'd buffer pointed | ||||
* to by pathbuf. The caller is responsible for free'ing the buffer from | * to by pathbuf. The caller is responsible for free'ing the buffer from | ||||
* the M_TEMP bucket if one is returned. | * the M_TEMP bucket if one is returned. | ||||
*/ | */ | ||||
int | int | ||||
kern_alternate_path(struct thread *td, const char *prefix, const char *path, | kern_alternate_path(struct thread *td, const char *prefix, const char *path, | ||||
Done Inline ActionsI might move this comment up to just above the function, and provide a little more explanation as to why this is needed, not just what it does. rwatson: I might move this comment up to just above the function, and provide a little more explanation… | |||||
Done Inline ActionsI can certainly do this. I was wondering, however, if this function would be considered useful or overly heavyweight: the call could be replaced by a conditional assignment in most places (and an unconditional assignment in one). I like the clarity of an explicit function call, but it can't really be inlined (unless I move the implementation to a header file, which I also don't like). jonathan: I can certainly do this. I was wondering, however, if this function would be considered useful… | |||||
Done Inline ActionsI would be OK with moving to various in situ -- especially of the form "... CAPMODE ? ENOTCAPABLE : EPERM". rwatson: I would be OK with moving to various in situ -- especially of the form "... CAPMODE ? | |||||
enum uio_seg pathseg, char **pathbuf, int create, int dirfd) | enum uio_seg pathseg, char **pathbuf, int create, int dirfd) | ||||
{ | { | ||||
struct nameidata nd, ndroot; | struct nameidata nd, ndroot; | ||||
Done Inline ActionsThis seems superfluous with the second KASSERT below drysdale_google.com: This seems superfluous with the second KASSERT below | |||||
Done Inline ActionsThis function will go away anyhow based the above discussion with Robert. jonathan: This function will go away anyhow based the above discussion with Robert. | |||||
char *ptr, *buf, *cp; | char *ptr, *buf, *cp; | ||||
size_t len, sz; | size_t len, sz; | ||||
int error; | int error; | ||||
buf = (char *) malloc(MAXPATHLEN, M_TEMP, M_WAITOK); | buf = (char *) malloc(MAXPATHLEN, M_TEMP, M_WAITOK); | ||||
*pathbuf = buf; | *pathbuf = buf; | ||||
/* Copy the prefix into the new pathname as a starting point. */ | /* Copy the prefix into the new pathname as a starting point. */ | ||||
len = strlcpy(buf, prefix, MAXPATHLEN); | len = strlcpy(buf, prefix, MAXPATHLEN); | ||||
if (len >= MAXPATHLEN) { | if (len >= MAXPATHLEN) { | ||||
*pathbuf = NULL; | *pathbuf = NULL; | ||||
free(buf, M_TEMP); | free(buf, M_TEMP); | ||||
return (EINVAL); | return (EINVAL); | ||||
} | } | ||||
Done Inline ActionsPossibly this should be a panic(), as should this occur at runtime, we want to fail closed, not open? Also, do include the proposed error number in the panic message. rwatson: Possibly this should be a panic(), as should this occur at runtime, we want to fail closed, not… | |||||
sz = MAXPATHLEN - len; | sz = MAXPATHLEN - len; | ||||
ptr = buf + len; | ptr = buf + len; | ||||
/* Append the filename to the prefix. */ | /* Append the filename to the prefix. */ | ||||
if (pathseg == UIO_SYSSPACE) | if (pathseg == UIO_SYSSPACE) | ||||
error = copystr(path, ptr, sz, &len); | error = copystr(path, ptr, sz, &len); | ||||
else | else | ||||
error = copyinstr(path, ptr, sz, &len); | error = copyinstr(path, ptr, sz, &len); | ||||
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines |
The caller is free to specify any error they want, yet this checks specifically for EPERM. I would say that's inconsequential for no good reason.
Instead, you can if (ndp->ni_nonrelativeerrno == 0) ndp->ni_nonrelativeerrno = ENOTCAPABLE;
while a no-op change with current consumers, may come in handy.