Page MenuHomeFreeBSD
Differential D2808 Diff 7189 sys/kern/vfs_syscalls.c

Changeset View

Standalone View

View Options

sys/kern/vfs_syscalls.c

Show First 20 Lines • Show All 953 Lines • ▼ Show 20 Lineskern_openat(struct thread *td, int fd, char *path, enum uio_seg pathseg,
int flags, int mode) int flags, int mode)
{ {
struct proc *p = td->td_proc; struct proc *p = td->td_proc;
struct filedesc *fdp = p->p_fd; struct filedesc *fdp = p->p_fd;
struct file *fp; struct file *fp;
struct vnode *vp; struct vnode *vp;
struct nameidata nd; struct nameidata nd;
cap_rights_t rights; cap_rights_t rights;
int cmode, error, indx; int beneath, cmode, error, indx;
indx = -1; indx = -1;
AUDIT_ARG_FFLAGS(flags); AUDIT_ARG_FFLAGS(flags);
AUDIT_ARG_MODE(mode); AUDIT_ARG_MODE(mode);
/* XXX: audit dirfd */ /* XXX: audit dirfd */
cap_rights_init(&rights, CAP_LOOKUP); cap_rights_init(&rights, CAP_LOOKUP);
flags_to_rights(flags, &rights); flags_to_rights(flags, &rights);
beneath = ((flags & O_BENEATH) == O_BENEATH);
/* /*
* Only one of the O_EXEC, O_RDONLY, O_WRONLY and O_RDWR flags * Only one of the O_EXEC, O_RDONLY, O_WRONLY and O_RDWR flags
* may be specified. * may be specified.
*/ */
if (flags & O_EXEC) { if (flags & O_EXEC) {
if (flags & O_ACCMODE) if (flags & O_ACCMODE)
return (EINVAL); return (EINVAL);
} else if ((flags & O_ACCMODE) == O_ACCMODE) { } else if ((flags & O_ACCMODE) == O_ACCMODE) {
return (EINVAL); return (EINVAL);
} else { } else {
flags = FFLAGS(flags); flags = FFLAGS(flags);
} }
drysdale_google.comUnsubmitted
Done
Inline Actions

(Random aside: does FreeBSD not attempt to police unrecognized flags? In Linux, syscalls are (now) recommended to reject unknown flags with EINVAL.)

drysdale_google.com: (Random aside: does FreeBSD not attempt to police unrecognized flags? In Linux, syscalls are…
jonathanAuthorUnsubmitted
Done
Inline Actions

This strikes me as a fundamentally good idea, but others may disagree for compatibility reasons... maybe worth talking about for 11, but probably not in this hopefully-MFC'able change.

jonathan: This strikes me as a fundamentally good idea, but others may disagree for compatibility reasons.
rwatsonUnsubmitted
Done
Inline Actions

I think validating flags is a good idea -- but agree that it might be done on a different MFC schedule and hence should be a separate change.

rwatson: I think validating flags is a good idea -- but agree that it might be done on a different MFC…
/* /*
* Allocate the file descriptor, but don't install a descriptor yet. * Allocate the file descriptor, but don't install a descriptor yet.
*/ */
error = falloc_noinstall(td, &fp); error = falloc_noinstall(td, &fp);
if (error != 0) if (error != 0)
return (error); return (error);
/* /*
* An extra reference on `fp' has been held for us by * An extra reference on `fp' has been held for us by
* falloc_noinstall(). * falloc_noinstall().
*/ */
/* Set the flags early so the finit in devfs can pick them up. */ /* Set the flags early so the finit in devfs can pick them up. */
fp->f_flag = flags & FMASK; fp->f_flag = flags & FMASK;
cmode = ((mode & ~fdp->fd_cmask) & ALLPERMS) & ~S_ISTXT; cmode = ((mode & ~fdp->fd_cmask) & ALLPERMS) & ~S_ISTXT;
NDINIT_ATRIGHTS(&nd, LOOKUP, FOLLOW | AUDITVNODE1, pathseg, path, fd, NDINIT_ATRIGHTS(&nd, LOOKUP, FOLLOW | AUDITVNODE1, pathseg, path, fd,
&rights, td); &rights, td);
if (beneath)
nd.ni_nonrelativeerrno = EPERM;
emasteUnsubmitted
Done
Inline Actions

Extra braces

emaste: Extra braces
jonathanAuthorUnsubmitted
Done
Inline Actions

Ok, I will change this conform to style(9), but I officially protest that I do so under duress. :)

From now on, I will refer to this paragraph of style(9) as the "goto fail" paragraph.

jonathan: Ok, I will change this conform to `style(9)`, but I officially protest that I do so under…
td->td_dupfd = -1; /* XXX check for fdopen */ td->td_dupfd = -1; /* XXX check for fdopen */
error = vn_open(&nd, &flags, cmode, fp); error = vn_open(&nd, &flags, cmode, fp);
if (error != 0) { if (error != 0) {
/* /*
* If the vn_open replaced the method vector, something * If the vn_open replaced the method vector, something
* wonderous happened deep below and we just pass it up * wonderous happened deep below and we just pass it up
* pretending we know what we do. * pretending we know what we do.
*/ */
if (error == ENXIO && fp->f_ops != &badfileops) if (error == ENXIO && fp->f_ops != &badfileops)
goto success; goto success;
/* /*
* Handle special fdopen() case. bleh. * Handle special fdopen() case. bleh.
* *
* Don't do this for relative (capability) lookups; we don't * Don't do this for capability or O_BENEATH lookups: we don't
* understand exactly what would happen, and we don't think * understand exactly what would happen, and we don't think
* that it ever should. * that it ever should.
*
* XXXJA: can fdopen() ever be called with O_BENEATH?
rwatsonUnsubmitted
Done
Inline Actions

Hmm. I think failing closed here is reasonable for capability mode -- but I'm not sure if that's true for ambient O_BENEATH. I believe td_dupfd is set for /dev/fd/[012] and /dev/{stderr,stdin,stdout}, which might reasonable be expected to work outside of capability mode. It also looks like it's used in another edge case or two (sysvr4 compatibility!?).

rwatson: Hmm. I think failing closed here is reasonable for capability mode -- but I'm not sure if…
jonathanAuthorUnsubmitted
Done
Inline Actions

Since we've said that O_BENEATHsupercedes the capability rules (in the sense that we want the O_BENEATH errno whether we're in or out of capability mode), we can't know at this point whether the original file was a capability: we just see EPERM.

If this code is related to fdopen(), I don't anticipate O_BENEATH being used much...

jonathan: Since we've said that `O_BENEATH`supercedes the capability rules (in the sense that we want the…
rwatsonUnsubmitted
Done
Inline Actions

Maybe make this an XXXCAP or XXXJA or such, but otherwise, yes, I agree. We'll see if anyone ever runs into it!

rwatson: Maybe make this an XXXCAP or XXXJA or such, but otherwise, yes, I agree. We'll see if anyone…
jonathanAuthorUnsubmitted
Done
Inline Actions

Adding more XXX here sounds like a fine thing to do. :)

jonathan: Adding more `XXX` here sounds like a fine thing to do. :)
*/ */
if (nd.ni_strictrelative == 0 && if (nd.ni_nonrelativeerrno == 0 &&
(error == ENODEV || error == ENXIO) && (error == ENODEV || error == ENXIO) &&
td->td_dupfd >= 0) { td->td_dupfd >= 0) {
error = dupfdopen(td, fdp, td->td_dupfd, flags, error, error = dupfdopen(td, fdp, td->td_dupfd, flags, error,
&indx); &indx);
if (error == 0) if (error == 0)
goto success; goto success;
} }
Show All 29 Lines
success: success:
/* /*
* If we haven't already installed the FD (for dupfdopen), do so now. * If we haven't already installed the FD (for dupfdopen), do so now.
*/ */
if (indx == -1) { if (indx == -1) {
struct filecaps *fcaps; struct filecaps *fcaps;
#ifdef CAPABILITIES #ifdef CAPABILITIES
if (nd.ni_strictrelative == 1) if (nd.ni_nonrelativeerrno != 0 &&
cap_rights_is_valid(&nd.ni_filecaps.fc_rights))
fcaps = &nd.ni_filecaps; fcaps = &nd.ni_filecaps;
else else
#endif #endif
fcaps = NULL; fcaps = NULL;
error = finstall(td, fp, &indx, flags, fcaps); error = finstall(td, fp, &indx, flags, fcaps);
/* On success finstall() consumes fcaps. */ /* On success finstall() consumes fcaps. */
if (error != 0) { if (error != 0) {
filecaps_free(&nd.ni_filecaps); filecaps_free(&nd.ni_filecaps);
▲ Show 20 Lines • Show All 3,598 Lines • Show Last 20 Lines