PERFORCE change 180327 for review
Efstratios Karatzas
gpf at FreeBSD.org
Tue Jun 29 21:26:05 UTC 2010
http://p4web.freebsd.org/@@180327?ac=10
Change 180327 by gpf at gpf_desktop on 2010/06/29 21:25:59
- fixed a minor bug in nfsrv_auditpath() wrapper function. We don't
really need to have a file handle to get the parent of the vnode
if e.g. our fs is ZFS, VOP_GETPARENT() does require a 'hint'.
- found & fixed an interesting bug: I hadn't spotted this before because
I wasn't auditing the class 'ad' (administrative), but now I saw that
auditing this class causes my code to kernel panic. The kernel
thread that traps via the nfssvc(2) syscall never really exits. It
stays in the kernel as nfsd and services various NFS RPCs. So it
never really audit_syscall_exit()s and the first time it will
try to service an RPC and audit_nfs_enter(), it will kernel panic
because we already have an active td_ar. Providing audit support
for multiple simultaneous td_ars does not solve this problem so
I applied a fix of sorts, please refer to the comment+code in
sys/nfs/nfs_nfssvc.c.
- minor typo fixes in /etc/audit_event
- tried to add audit support for a few nfsv4 ops and hit a wall.
I'm tired of saying that auditing paths from vnodes for UFS requires
a connection between vnode + parent directory that contains it. For
NFSv 2 & 3, we keep a 'hint' directory inode. This hint is kept with
the file handle that is sent back to the client, et voila.
NFSv4 has a peculiar way of doing a lot of things. For example, say we
wish to acquire the fh of a regular file. This is the compound RPC:
putfh(directory filehandle), lookup(file entry name), getfh().
Note that in NFSv4, lookup() is supposed to change the 'current file
handle' (to that of the file that is looked up), and getfh() is supposed
to return the file handle via the reply buffer of the RPC.
In FreeBSD's nfsv4 implementation, the 'current' and saved file handles'
are actually current and saved vnode pointers. The author
notes that it might be cleaner to save & use fhs instead of vps.
So, although NFSv3 would return a filehandle inside lookup(), NFSv4
will return the filehandle through getfh(). getfh() should push
the current filehandle into the reply buffer of the rpc. But since
the current filehandle is actually a vnode pointer, it can only
do a VOP_VPTOFH() and therefore, not save the beforementioned hint.
To put it simply, we can't audit file paths when the namecache fails
us, because of the way nfsv4 is implemented. It will require some
considerable change to work the way I want it to.
Note: excuse the lengthy description but I'm writing this in hopes of
my mentor and Rick Macklem one day reading this.
Thank you
Affected files ...
.. //depot/projects/soc2010/gpf_audit/freebsd/src/contrib/openbsm/etc/audit_event#4 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsocket.c#9 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsubs.c#3 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfs/nfs_nfssvc.c#2 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_serv.c#17 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#8 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit_bsm.c#11 edit
Differences ...
==== //depot/projects/soc2010/gpf_audit/freebsd/src/contrib/openbsm/etc/audit_event#4 (text) ====
@@ -391,7 +391,7 @@
2023:AUE_NFS_CLOSE:nfsrv_close():cl
2024:AUE_NFS_DELEGPURGE:nfsrv_delegpurge():ad
2025:AUE_NFS_DELEGRETURN:nfsrv_delegreturn():ad
-2026:AUE_NFSv4_GETFH:nfsrv_getfh():ad
+2026:AUE_NFSv4_GETFH:nfsrv4_getfh():ad
2027:AUE_NFS_LOCK:nfsrv_lock():fm
2028:AUE_NFS_LOCKT:nfsrv_lockt():fm
2029:AUE_NFS_LOCKU:nfsrv_locku():fm
@@ -403,7 +403,7 @@
2035:AUE_NFS_OPENDOWNGRADE:nfsrv_opendowngrade():fm
2036:AUE_NFS_PUTFH:nfsrv_putfh():ad
2037:AUE_NFS_PUTPUBFH:nfsrv_putpubfh():ad
-2038:AUE_NFS_PUTROOTFH:nfsrv_rootfh():ad
+2038:AUE_NFS_PUTROOTFH:nfsrv_putrootfh():ad
2039:AUE_NFS_RENEW:nfsrv_renew():ad
2040:AUE_NFS_RESTOREFH:nfsrv_restorefh():ad
2041:AUE_NFS_SAVEFH:nfsrv_savefh():ad
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsocket.c#9 (text+ko) ====
@@ -436,6 +436,7 @@
nfsrvd_compound(nd, isdgram, p);
printf("compound rpc exit\n");
} else {
+ printf("non compound rpc %d\n", nd->nd_procnum);
if (nd->nd_flag & ND_NFSV2)
nfsprot = ND_NFSV2;
else
@@ -736,14 +737,19 @@
}
if (nfsv4_opflag[op].savereply)
nd->nd_flag |= ND_SAVEREPLY;
- NFSINCRGLOBAL(newnfsstats.srvrpccnt[nd->nd_procnum]);
+ NFSINCRGLOBAL(newnfsstats.srvrpccnt[nd->nd_procnum]);
+ AUDIT_NFS_ENTER(op, nd->nd_cred, curthread, ND_NFSV4);
+ if (nd->nd_nam != NULL)
+ AUDIT_ARG_SOCKADDR_IN((struct sockaddr_in *)nd->nd_nam);
switch (op) {
/* xxx gpf */
printf("op = %d\n", op);
case NFSV4OP_PUTFH:
error = nfsrv_mtofh(nd, &fh);
- if (error)
+ if (error) {
+ printf("error! %p\n", curthread);
goto nfsmout;
+ }
if (!nd->nd_repstat) {
nes.nes_vfslocked = vpnes.nes_vfslocked;
nfsd_fhtovp(nd, &fh, &nvp, &nes, &mp,
@@ -754,7 +760,12 @@
if (vp)
vrele(vp);
vp = nvp;
+ vref(vp);
+ AUDIT_ARG_VNODE1(vp);
NFSVOPUNLOCK(vp, 0, p);
+ nfsrv_auditpath(vp, NULL, NULL,
+ (fhandle_t *)fh.nfsrvfh_data, 1);
+ vrele(vp);
vpnes = nes;
}
break;
@@ -770,7 +781,12 @@
if (vp)
vrele(vp);
vp = nvp;
+ vref(vp);
+ AUDIT_ARG_VNODE1(vp);
NFSVOPUNLOCK(vp, 0, p);
+ nfsrv_auditpath(vp, NULL, NULL,
+ (fhandle_t *)nfs_pubfh.nfsrvfh_data, 1);
+ vrele(vp);
vpnes = nes;
}
break;
@@ -783,7 +799,12 @@
if (vp)
vrele(vp);
vp = nvp;
+ vref(vp);
+ AUDIT_ARG_VNODE1(vp);
NFSVOPUNLOCK(vp, 0, p);
+ nfsrv_auditpath(vp, NULL, NULL,
+ (fhandle_t *)nfs_rootfh.nfsrvfh_data, 1);
+ vrele(vp);
vpnes = nes;
}
} else if (nfsv4root_vp && nfsv4root_set) {
@@ -813,6 +834,14 @@
savevpnes = vpnes;
savemp = mp;
}
+ /* XXXgpf: is this the correct filehandle? */
+ if (savevp) {
+ nfsrv_auditpath(savevp, NULL, NULL,
+ (fhandle_t *)fh.nfsrvfh_data, 1);
+ vn_lock(savevp, LK_EXCLUSIVE);
+ AUDIT_ARG_VNODE1(savevp);
+ VOP_UNLOCK(savevp, 0);
+ }
} else {
nd->nd_repstat = NFSERR_NOFILEHANDLE;
}
@@ -820,6 +849,14 @@
case NFSV4OP_RESTOREFH:
if (savevp) {
nd->nd_repstat = 0;
+ /* XXXgpf: file handle? */
+ vref(savevp);
+ nfsrv_auditpath(savevp, NULL, NULL,
+ NULL, 1);
+ vn_lock(savevp, LK_EXCLUSIVE);
+ AUDIT_ARG_VNODE1(savevp);
+ VOP_UNLOCK(savevp, 0);
+ vrele(savevp);
/* If vp == savevp, a no-op */
if (vp != savevp) {
VREF(savevp);
@@ -945,8 +982,16 @@
if (nfsv4_opflag[op].modifyfs)
NFS_STARTWRITE(NULL, &mp);
NFSVOPLOCK(savevp, LK_EXCLUSIVE | LK_RETRY, p);
+ if (savevp)
+ vref(savevp);
error = (*(nfsrv4_ops2[op]))(nd, isdgram, savevp,
vp, p, &savevpnes, &vpnes);
+ if (savevp) {
+ if (nd->nd_procnum == NFSPROC_LINK)
+ nfsrv_auditpath(savevp, NULL, NULL,
+ (fhandle_t *)fh.nfsrvfh_data, 2);
+ vrele(savevp);
+ }
if (nfsv4_opflag[op].modifyfs)
NFS_ENDWRITE(mp);
} else {
@@ -981,6 +1026,7 @@
}
}
};
+ AUDIT_NFS_EXIT(nd->nd_repstat, curthread);
if (error) {
if (error == EBADRPC || error == NFSERR_BADXDR) {
nd->nd_repstat = NFSERR_BADXDR;
@@ -999,6 +1045,9 @@
}
}
nfsmout:
+ /* XXXgpf: when error occurs, we may jump here */
+ AUDIT_NFS_EXIT(nd->nd_repstat, curthread);
+ KASSERT(curthread->td_ar == NULL, ("gamw sto nfsmout: td->td_ar != NULL"));
if (error) {
if (error == EBADRPC || error == NFSERR_BADXDR)
nd->nd_repstat = NFSERR_BADXDR;
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsubs.c#3 (text+ko) ====
@@ -2061,6 +2061,9 @@
* fname - name used to reference vp inside dvp
* fhp - file handle for vp
* n - AUDIT_ARG_UPATH1 or AUDIT_ARG_UPATH2
+ *
+ * Note: vp and dvp may be vref'd but not locks should be held on them as the
+ * two vn_fullpath_* KPIs may try to lock them themselves.
*/
void
nfsrv_auditpath(vnode_t vp, vnode_t dvp, char *fname, fhandle_t *fhp, int n)
@@ -2085,11 +2088,13 @@
success = 1;
goto out;
}
+
/* if our cache fails us */
- if (fhp != NULL && vp->v_mount != NULL) {
+ if (vp->v_mount != NULL) {
uint64_t parent_hint;
/* get the hint stored inside the file handle */
- VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
+ if (fhp != NULL)
+ VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
vn_fullpath_nocache(vp, &fullpath, &freepath,
parent_hint, PARENTHINT | WANTNAME);
if (freepath != NULL) {
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfs/nfs_nfssvc.c#2 (text+ko) ====
@@ -79,7 +79,18 @@
KASSERT(!mtx_owned(&Giant), ("nfssvc(): called with Giant"));
- AUDIT_ARG_CMD(uap->flag);
+ AUDIT_ARG_CMD(uap->flag);
+ /*
+ * XXX:gpf quick fix until I figure out something better.
+ * Even if Audit supports multiple simultaneous td_ars per
+ * kernel thread, we still want to end the audit record for
+ * nfs_svc so that Audit records for NFS RPCs that are serviced
+ * by this thread are recorded without any trouble such as waiting
+ * for nfssvc() to return before commiting them to the audit daemon.
+ * Problem is that we are now *not* recording the return value from
+ * the nfs server.
+ */
+ AUDIT_SYSCALL_EXIT(0, td);
error = priv_check(td, PRIV_NFS_DAEMON);
if (error)
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_serv.c#17 (text+ko) ====
@@ -203,6 +203,9 @@
* fname - name used to reference vp inside dvp
* fhp - file handle for vp
* n - AUDIT_ARG_UPATH1 or AUDIT_ARG_UPATH2
+ *
+ * Note: vp and dvp may be vref'd but not locks should be held on them as the
+ * two vn_fullpath_* KPIs may try to lock them themselves.
*/
static void
nfsrv_auditpath(struct vnode *vp, struct vnode *dvp, char *fname, fhandle_t *fhp, int n)
@@ -226,10 +229,11 @@
}
/* if our cache fails us */
- if (fhp != NULL && vp->v_mount != NULL) {
+ if (vp->v_mount != NULL) {
uint64_t parent_hint;
/* get the hint stored inside the file handle */
- VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
+ if (fhp != NULL)
+ VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
vn_fullpath_nocache(vp, &fullpath, &freepath,
parent_hint, PARENTHINT | WANTNAME);
if (freepath != NULL) {
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#8 (text) ====
@@ -706,6 +706,9 @@
au_id_t auid;
int error;
+ if (td->td_ar != NULL) {
+ printf("bug event = %d\n", td->td_ar->k_ar.ar_event);
+ }
KASSERT(td->td_ar == NULL, ("audit_nfs_enter: td->td_ar != NULL"));
KASSERT((td->td_pflags & TDP_AUDITREC) == 0,
("audit_nfs_enter: TDP_AUDITREC set"));
@@ -797,7 +800,6 @@
audit_nfs_exit(int error, struct thread *td)
{
int retval;
-
/*
* Commit the audit record as desired; once we pass the record into
* audit_commit(), the memory is owned by the audit subsystem. The
==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit_bsm.c#11 (text) ====
@@ -1648,6 +1648,44 @@
}
break;
+ case AUE_NFS_PUTFH:
+ case AUE_NFS_PUTPUBFH:
+ case AUE_NFS_PUTROOTFH:
+ case AUE_NFS_RESTOREFH:
+ case AUE_NFS_SAVEFH:
+ UPATH1_VNODE1_TOKENS;
+ if (ARG_IS_VALID(kar, ARG_TEXT)) {
+ tok = au_to_text(ar->ar_arg_text);
+ kau_write(rec, tok);
+ }
+ break;
+
+ /* XXXgpf: temporary fallthrough for nfsv4 events */
+ case AUE_NFS_CLOSE:
+ case AUE_NFS_DELEGPURGE:
+ case AUE_NFS_DELEGRETURN:
+ case AUE_NFSv4_GETFH:
+ case AUE_NFS_LOCK:
+ case AUE_NFS_LOCKT:
+ case AUE_NFS_LOCKU:
+ case AUE_NFS_LOOKUPP:
+ case AUE_NFS_NVERIFY:
+ case AUE_NFS_OPEN:
+ case AUE_NFS_OPENATTR:
+ case AUE_NFS_OPENCONFIRM:
+ case AUE_NFS_OPENDOWNGRADE:
+ case AUE_NFS_RENEW:
+ case AUE_NFS_SECINFO:
+ case AUE_NFS_SETCLIENTID:
+ case AUE_NFS_SETCLIENTIDCFRM:
+ case AUE_NFS_VERIFY:
+ case AUE_NFS_RELEASELCKOWN:
+ if (ARG_IS_VALID(kar, ARG_TEXT)) {
+ tok = au_to_text(ar->ar_arg_text);
+ kau_write(rec, tok);
+ }
+ break;
+
case AUE_WAIT4:
PROCESS_PID_TOKENS(1);
if (ARG_IS_VALID(kar, ARG_VALUE)) {
More information about the p4-projects
mailing list