Page MenuHomeFreeBSD
Differential D24596 Diff 71826 sys/kern/vfs_vnops.c

Changeset View

Standalone View

View Options

sys/kern/vfs_vnops.c

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 LinesSYSCTL_INT(_debug, OID_AUTO, vn_io_fault_enable, CTLFLAG_RW,
&vn_io_fault_enable, 0, "Enable vn_io_fault lock avoidance"); &vn_io_fault_enable, 0, "Enable vn_io_fault lock avoidance");
static int vn_io_fault_prefault = 0; static int vn_io_fault_prefault = 0;
SYSCTL_INT(_debug, OID_AUTO, vn_io_fault_prefault, CTLFLAG_RW, SYSCTL_INT(_debug, OID_AUTO, vn_io_fault_prefault, CTLFLAG_RW,
&vn_io_fault_prefault, 0, "Enable vn_io_fault prefaulting"); &vn_io_fault_prefault, 0, "Enable vn_io_fault prefaulting");
static u_long vn_io_faults_cnt; static u_long vn_io_faults_cnt;
SYSCTL_ULONG(_debug, OID_AUTO, vn_io_faults, CTLFLAG_RD, SYSCTL_ULONG(_debug, OID_AUTO, vn_io_faults, CTLFLAG_RD,
&vn_io_faults_cnt, 0, "Count of vn_io_fault lock avoidance triggers"); &vn_io_faults_cnt, 0, "Count of vn_io_fault lock avoidance triggers");
static int vfs_allow_read_dir = 0;
SYSCTL_INT(_security_bsd, OID_AUTO, allow_read_dir, CTLFLAG_RW,
&vfs_allow_read_dir, 0,
"Enable read(2) of directory by root for filesystems that support it");
/* /*
* Returns true if vn_io_fault mode of handling the i/o request should * Returns true if vn_io_fault mode of handling the i/o request should
* be used. * be used.
*/ */
static bool static bool
do_vn_io_fault(struct vnode *vp, struct uio *uio) do_vn_io_fault(struct vnode *vp, struct uio *uio)
{ {
struct mount *mp; struct mount *mp;
▲ Show 20 Lines • Show All 1,009 Lines • ▼ Show 20 Linesvn_io_fault(struct file *fp, struct uio *uio, struct ucred *active_cred,
fo_rdwr_t *doio; fo_rdwr_t *doio;
struct vnode *vp; struct vnode *vp;
void *rl_cookie; void *rl_cookie;
struct vn_io_fault_args args; struct vn_io_fault_args args;
int error; int error;
doio = uio->uio_rw == UIO_READ ? vn_read : vn_write; doio = uio->uio_rw == UIO_READ ? vn_read : vn_write;
vp = fp->f_vnode; vp = fp->f_vnode;
/*
* The ability to read(2) on a directory has historically been
* allowed for all users, but this can and has been the source of
* at least one security issue in the past. As such, it is now hidden
* away behind a sysctl for those that actually need it to use it, and
* restricted to root when it's turned on to make it relatively safe to
* leave on for longer sessions of need.
*/
kibUnsubmitted
Done
Inline Actions

There should be a blank line after the last line of code that is described by multi-line comment, style(9).

kib: There should be a blank line after the last line of code that is described by multi-line…
kevansAuthorUnsubmitted
Done
Inline Actions

Fixed in my local copy, will roll it into next update if others request changes.

kevans: Fixed in my local copy, will roll it into next update if others request changes.
bdreweryUnsubmitted
Not Done
Inline Actions

Where is this in style(9)?

bdrewery: Where is this in style(9)?
kibUnsubmitted
Not Done
Inline Actions

I cannot find it in style(9), but I also cannot find there a rule that multi-line comment should be preceded by a blank line.

kib: I cannot find it in style(9), but I also cannot find there a rule that multi-line comment…
bdreweryUnsubmitted
Not Done
Inline Actions

So are we saying the style should be this?

code_thing_A;
code_thing_B;

/*
  * Comment describing code doing thing C
  */
code_thing_C;

code_thing_D;
code_thing_E;

I can add an example of this to style(9) if that's what you're saying. The blank line rules always confused me.

bdrewery: So are we saying the style should be this? ``` code_thing_A; code_thing_B; /* * Comment…
kibUnsubmitted
Not Done
Inline Actions

Yes, I definitely was told to use this style very early. I do not remember, was it bde or alc.

The corner cases are the following:

statement;
something (if/for...) {
      /*
        * Multi-line, no blank line before.
        */
        statement;

and

     statement;

     /*
       * Multi-line.
       */
}
statement;

so no blank line after '{ and multi-line, and no blank line after multi-line if next is '}'.
I think sys/vm is more or less consistent in this rule, at least.

kib: Yes, I definitely was told to use this style very early. I do not remember, was it bde or alc.
if (vp->v_type == VDIR) {
KASSERT(uio->uio_rw == UIO_READ,
("illegal write attempted on a directory"));
rgrimesUnsubmitted
Not Done
Inline Actions

This is not described in the summary of what you are changing. I really fail why we need to panic the system if we reach this point, just return an error and prevent the write.

rgrimes: This is not described in the summary of what you are changing. I really fail why we need to…
kevansAuthorUnsubmitted
Done
Inline Actions

Ah, I'll mention it in the commit message... this isn't a situation that can reasonably happen, as it should be prevented higher up because you cannot open a directory writable. If we've reached this point with a directory/write, then something has gone horribly terribly wrong and it must be debugged.

kevans: Ah, I'll mention it in the commit message... this isn't a situation that can reasonably happen…
mckusickUnsubmitted
Not Done
Inline Actions

I agree with Kyle. Getting here with a write is a kernel logic error that should be debugged.

mckusick: I agree with Kyle. Getting here with a write is a kernel logic error that should be debugged.
if (!vfs_allow_read_dir)
return (EISDIR);
if ((error = priv_check(td, PRIV_VFS_READ_DIR)) != 0)
return (EISDIR);
}
foffset_lock_uio(fp, uio, flags); foffset_lock_uio(fp, uio, flags);
if (do_vn_io_fault(vp, uio)) { if (do_vn_io_fault(vp, uio)) {
args.kind = VN_IO_FAULT_FOP; args.kind = VN_IO_FAULT_FOP;
args.args.fop_args.fp = fp; args.args.fop_args.fp = fp;
args.args.fop_args.doio = doio; args.args.fop_args.doio = doio;
args.cred = active_cred; args.cred = active_cred;
args.flags = flags | FOF_OFFSET; args.flags = flags | FOF_OFFSET;
if (uio->uio_rw == UIO_READ) { if (uio->uio_rw == UIO_READ) {
▲ Show 20 Lines • Show All 2,015 Lines • Show Last 20 Lines