Page MenuHomeFreeBSD
Differential D25869 Diff 75175 sys/security/audit/audit_arg.c

Changeset View

Standalone View

View Options

sys/security/audit/audit_arg.c

Show First 20 Lines • Show All 1,013 Lines • ▼ Show 20 Lines if (getvnode(td, fd, cap_rights_init(&rights), &fp) != 0)
return; return;
vp = fp->f_vnode; vp = fp->f_vnode;
vn_lock(vp, LK_SHARED | LK_RETRY); vn_lock(vp, LK_SHARED | LK_RETRY);
audit_arg_vnode1(vp); audit_arg_vnode1(vp);
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
fdrop(fp, td); fdrop(fp, td);
} }
/* NFS RPC related audit args */
void
audit_nfsarg_dev(struct kaudit_record *ar, int dev)
{
if (ar == NULL)
return;
ar->k_ar.ar_arg_dev = dev;
ARG_SET_VALID(ar, ARG_DEV);
}
void
audit_nfsarg_mode(struct kaudit_record *ar, mode_t mode)
{
if (ar == NULL)
return;
ar->k_ar.ar_arg_mode = mode;
ARG_SET_VALID(ar, ARG_MODE);
}
void
audit_nfsarg_netsockaddr(struct kaudit_record *ar, struct sockaddr *sa)
{
KASSERT(sa != NULL, ("audit_nfsarg_sockaddr: sa == NULL"));
if (ar == NULL)
return;
bcopy(sa, &ar->k_ar.ar_arg_sockaddr, sa->sa_len);
switch (sa->sa_family) {
//#ifdef INET
case AF_INET:
ARG_SET_VALID(ar, ARG_SADDRINET);
break;
//#endif
//#ifdef INET6
case AF_INET6:
ARG_SET_VALID(ar, ARG_SADDRINET6);
asomersUnsubmitted
Done
Inline Actions

I recommend a KASSERT in the default position.

asomers: I recommend a KASSERT in the default position.
shivankAuthorUnsubmitted
Done
Inline Actions

I use panic(9) since no expression to evaluate?

shivank: I use panic(9) since no expression to evaluate?
asomersUnsubmitted
Done
Inline Actions

Good. Now we'll get to find out if somebody has figured out how to mount NFS over bluetooth ;)

asomers: Good. Now we'll get to find out if somebody has figured out how to mount NFS over bluetooth ;)
rmacklemUnsubmitted
Done
Inline Actions

bz@ likes anything based on AF_INET or AF_INET6 to be #ifdef INET
or #ifdef INET6 even if it builds without that.
(It documents where the code fragments are and he dreams of getting
rid of INET someday, although I doubt that will be in my lifetime.)

rmacklem: bz@ likes anything based on AF_INET or AF_INET6 to be #ifdef INET or #ifdef INET6 even if it…
shivankAuthorUnsubmitted
Done
Inline Actions

I did the #ifdef INET... #endif part. But something is wrong somewhere as it skips the block and jumps to the default case. (pointing the INET/INET6 is not defined)
Am I missing some headers file?

shivank: I did the `#ifdef INET... #endif` part. But something is wrong somewhere as it skips the block…
break;
//#endif
default:
printf/*panic*/("audit_nfsarg_netsockaddr: invalid sa_family");
}
}
void
audit_nfsarg_socket(struct kaudit_record *ar, int sodomain, int sotype,
int soprotocol)
{
if (ar == NULL)
return;
ar->k_ar.ar_arg_sockinfo.so_domain = sodomain;
ar->k_ar.ar_arg_sockinfo.so_type = sotype;
ar->k_ar.ar_arg_sockinfo.so_protocol = soprotocol;
ARG_SET_VALID(ar, ARG_SOCKINFO);
}
void
audit_nfsarg_text(struct kaudit_record *ar, const char *text)
{
KASSERT(text != NULL, ("audit_arg_text: text == NULL"));
if (ar == NULL)
return;
/* Invalidate the text string */
ar->k_ar.ar_valid_arg &= (ARG_ALL ^ ARG_TEXT);
if (ar->k_ar.ar_arg_text == NULL)
ar->k_ar.ar_arg_text = malloc(MAXPATHLEN, M_AUDITTEXT,
M_WAITOK);
strncpy(ar->k_ar.ar_arg_text, text, MAXPATHLEN);
ARG_SET_VALID(ar, ARG_TEXT);
}
void
audit_nfsarg_upath1_vp(struct kaudit_record *ar, struct thread *td,
struct vnode *rdir, struct vnode *cdir, char *upath)
{
if (ar == NULL)
return;
KASSERT(VOP_ISLOCKED(cdir) == 0, ("audit_nfsarg_upath1_vp: vp locked"));
audit_arg_upath_vp(td, rdir, cdir, upath, &ar->k_ar.ar_arg_upath1);
ARG_SET_VALID(ar, ARG_UPATH1);
}
void
audit_nfsarg_upath2_vp(struct kaudit_record *ar, struct thread *td,
struct vnode *rdir, struct vnode *cdir, char *upath)
{
if (ar == NULL)
return;
KASSERT(VOP_ISLOCKED(cdir) == 0, ("audit_nfsarg_upath1_vp: vp locked"));
audit_arg_upath_vp(td, rdir, cdir, upath, &ar->k_ar.ar_arg_upath2);
ARG_SET_VALID(ar, ARG_UPATH2);
}
void
audit_nfsarg_value(struct kaudit_record *ar, long value)
{
if(ar == NULL)
return;
ar->k_ar.ar_arg_value = value;
ARG_SET_VALID(ar,ARG_VALUE);
}
void
audit_nfsarg_vnode1(struct kaudit_record *ar, struct vnode *vp)
{
int error, lktype = 0;
/* Page fault panic occur if vnode *vp is NULL. */
KASSERT(vp != NULL, ("audit_nfsarg_vnode1: vp == NULL"));
if (ar == NULL)
return;
lktype = VOP_ISLOCKED(vp);
/*XXX: audit_arg_vnode uses td_ucread. do we need nd_cr for NFS?*/
ARG_CLEAR_VALID(ar, ARG_VNODE1);
/* Hold the vnode lock for VOP_GETTR call. */
if (lktype != LK_EXCLUSIVE && lktype != LK_SHARED) {
vref(vp);
if (vn_lock(vp, LK_SHARED | LK_NOWAIT)) {
vrele(vp);
return;
}
}
error = audit_arg_vnode(vp, &ar->k_ar.ar_arg_vnode1);
if (lktype != LK_EXCLUSIVE && lktype != LK_SHARED) {
vput(vp);
}
if (error == 0)
ARG_SET_VALID(ar, ARG_VNODE1);
}
rmacklemUnsubmitted
Done
Inline Actions

I'll admit I do not understand what this is trying to do.

I do not see any reason to vref()/vrele() the vnode, since
there must be a v_usecount held by the function where this
is called and that will not go away.

Also, VOP_ISLOCKED() returns the type of lock held.
The code needs to look something like:

locktype = VOP_ISLOCKED(vp);
/* As well as returning 0 for unlocked, it can return LK_EXCLOTHER for

another thread holding a lock on it. */

if (locktype != LK_EXCLUSIVE && locktype != LK_SHARED)

  • lock it

then if you locked it

VOP_UNLOCK(vp);

Then, if you do the vn_lock() without LK_NOWAIT as you have done now,
you must be careful to never call this when another vnode lock is held,
since ordering of vnode locking if you are to lock multiple vnodes must
be carefully designed to avoid deadlocks. (There are very few people in
the world, and I am not one of them, who actually understand how to safely
to lookup and rename of paths the intersect, as an example.)

The safe way to this, which I would be comfortable with, would be a

vn_lock(vp, LK_SHARE | LK_NOWAIT);
then if it fails you can't get all the audit information.

--> I'm guessing you would still want to generate an audit record, but it

won't be able to hold vnode details.
rmacklem: I'll admit I do not understand what this is trying to do. I do not see any reason to vref…
shivankAuthorUnsubmitted
Done
Inline Actions

if I don't use vref/vrele the kernel crashes - panic: condition vp->v_holdcnt > 0 not met at /usr/home/shivank/freebsd/sys/kern/vfs_vnops.c:1700 (_vn_lock)

I did the suggested changes for locking.

shivank: if I don't use vref/vrele the kernel crashes - panic: condition vp->v_holdcnt > 0 not met at…