Date: Fri, 30 Nov 2012 23:18:49 +0000 (UTC) From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r243726 - in head/sys: kern security/audit Message-ID: <201211302318.qAUNInIh074238@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: pjd Date: Fri Nov 30 23:18:49 2012 New Revision: 243726 URL: http://svnweb.freebsd.org/changeset/base/243726 Log: IFp4 @208451: Fix path handling for *at() syscalls. Before the change directory descriptor was totally ignored, so the relative path argument was appended to current working directory path and not to the path provided by descriptor, thus wrong paths were stored in audit logs. Now that we use directory descriptor in vfs_lookup, move AUDIT_ARG_UPATH1() and AUDIT_ARG_UPATH2() calls to the place where we hold file descriptors table lock, so we are sure paths will be resolved according to the same directory in audit record and in actual operation. Sponsored by: FreeBSD Foundation (auditdistd) Reviewed by: rwatson MFC after: 2 weeks Modified: head/sys/kern/vfs_lookup.c head/sys/security/audit/audit.c head/sys/security/audit/audit.h head/sys/security/audit/audit_arg.c head/sys/security/audit/audit_bsm_klib.c head/sys/security/audit/audit_private.h Modified: head/sys/kern/vfs_lookup.c ============================================================================== --- head/sys/kern/vfs_lookup.c Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/kern/vfs_lookup.c Fri Nov 30 23:18:49 2012 (r243726) @@ -160,17 +160,6 @@ namei(struct nameidata *ndp) error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN, (size_t *)&ndp->ni_pathlen); - if (error == 0) { - /* - * If we are auditing the kernel pathname, save the user - * pathname. - */ - if (cnp->cn_flags & AUDITVNODE1) - AUDIT_ARG_UPATH1(td, cnp->cn_pnbuf); - if (cnp->cn_flags & AUDITVNODE2) - AUDIT_ARG_UPATH2(td, cnp->cn_pnbuf); - } - /* * Don't allow empty pathnames. */ @@ -218,6 +207,14 @@ namei(struct nameidata *ndp) ndp->ni_rootdir = fdp->fd_rdir; ndp->ni_topdir = fdp->fd_jdir; + /* + * If we are auditing the kernel pathname, save the user pathname. + */ + if (cnp->cn_flags & AUDITVNODE1) + AUDIT_ARG_UPATH1(td, ndp->ni_dirfd, , cnp->cn_pnbuf); + if (cnp->cn_flags & AUDITVNODE2) + AUDIT_ARG_UPATH2(td, ndp->ni_dirfd, , cnp->cn_pnbuf); + dp = NULL; if (cnp->cn_pnbuf[0] != '/') { if (ndp->ni_startdir != NULL) { Modified: head/sys/security/audit/audit.c ============================================================================== --- head/sys/security/audit/audit.c Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/security/audit/audit.c Fri Nov 30 23:18:49 2012 (r243726) @@ -691,7 +691,7 @@ audit_proc_coredump(struct thread *td, c if (path != NULL) { pathp = &ar->k_ar.ar_arg_upath1; *pathp = malloc(MAXPATHLEN, M_AUDITPATH, M_WAITOK); - audit_canon_path(td, path, *pathp); + audit_canon_path(td, AT_FDCWD, path, *pathp); ARG_SET_VALID(ar, ARG_UPATH1); } ar->k_ar.ar_arg_signum = td->td_proc->p_sig; Modified: head/sys/security/audit/audit.h ============================================================================== --- head/sys/security/audit/audit.h Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/security/audit/audit.h Fri Nov 30 23:18:49 2012 (r243726) @@ -99,8 +99,8 @@ void audit_arg_sockaddr(struct thread * void audit_arg_auid(uid_t auid); void audit_arg_auditinfo(struct auditinfo *au_info); void audit_arg_auditinfo_addr(struct auditinfo_addr *au_info); -void audit_arg_upath1(struct thread *td, char *upath); -void audit_arg_upath2(struct thread *td, char *upath); +void audit_arg_upath1(struct thread *td, int dirfd, char *upath); +void audit_arg_upath2(struct thread *td, int dirfd, char *upath); void audit_arg_vnode1(struct vnode *vp); void audit_arg_vnode2(struct vnode *vp); void audit_arg_text(char *text); @@ -276,14 +276,14 @@ void audit_thread_free(struct thread *t audit_arg_uid((uid)); \ } while (0) -#define AUDIT_ARG_UPATH1(td, upath) do { \ +#define AUDIT_ARG_UPATH1(td, dirfd, upath) do { \ if (AUDITING_TD(curthread)) \ - audit_arg_upath1((td), (upath)); \ + audit_arg_upath1((td), (dirfd), (upath)); \ } while (0) -#define AUDIT_ARG_UPATH2(td, upath) do { \ +#define AUDIT_ARG_UPATH2(td, dirfd, upath) do { \ if (AUDITING_TD(curthread)) \ - audit_arg_upath2((td), (upath)); \ + audit_arg_upath2((td), (dirfd), (upath)); \ } while (0) #define AUDIT_ARG_VALUE(value) do { \ @@ -356,8 +356,8 @@ void audit_thread_free(struct thread *t #define AUDIT_ARG_SUID(suid) #define AUDIT_ARG_TEXT(text) #define AUDIT_ARG_UID(uid) -#define AUDIT_ARG_UPATH1(td, upath) -#define AUDIT_ARG_UPATH2(td, upath) +#define AUDIT_ARG_UPATH1(td, dirfd, upath) +#define AUDIT_ARG_UPATH2(td, dirfd, upath) #define AUDIT_ARG_VALUE(value) #define AUDIT_ARG_VNODE1(vp) #define AUDIT_ARG_VNODE2(vp) Modified: head/sys/security/audit/audit_arg.c ============================================================================== --- head/sys/security/audit/audit_arg.c Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/security/audit/audit_arg.c Fri Nov 30 23:18:49 2012 (r243726) @@ -463,7 +463,8 @@ audit_arg_sockaddr(struct thread *td, st break; case AF_UNIX: - audit_arg_upath1(td, ((struct sockaddr_un *)sa)->sun_path); + audit_arg_upath1(td, AT_FDCWD, + ((struct sockaddr_un *)sa)->sun_path); ARG_SET_VALID(ar, ARG_SADDRUNIX); break; /* XXXAUDIT: default:? */ @@ -707,16 +708,16 @@ audit_arg_file(struct proc *p, struct fi * freed when the audit record is freed. */ static void -audit_arg_upath(struct thread *td, char *upath, char **pathp) +audit_arg_upath(struct thread *td, int dirfd, char *upath, char **pathp) { if (*pathp == NULL) *pathp = malloc(MAXPATHLEN, M_AUDITPATH, M_WAITOK); - audit_canon_path(td, upath, *pathp); + audit_canon_path(td, dirfd, upath, *pathp); } void -audit_arg_upath1(struct thread *td, char *upath) +audit_arg_upath1(struct thread *td, int dirfd, char *upath) { struct kaudit_record *ar; @@ -724,12 +725,12 @@ audit_arg_upath1(struct thread *td, char if (ar == NULL) return; - audit_arg_upath(td, upath, &ar->k_ar.ar_arg_upath1); + audit_arg_upath(td, dirfd, upath, &ar->k_ar.ar_arg_upath1); ARG_SET_VALID(ar, ARG_UPATH1); } void -audit_arg_upath2(struct thread *td, char *upath) +audit_arg_upath2(struct thread *td, int dirfd, char *upath) { struct kaudit_record *ar; @@ -737,7 +738,7 @@ audit_arg_upath2(struct thread *td, char if (ar == NULL) return; - audit_arg_upath(td, upath, &ar->k_ar.ar_arg_upath2); + audit_arg_upath(td, dirfd, upath, &ar->k_ar.ar_arg_upath2); ARG_SET_VALID(ar, ARG_UPATH2); } Modified: head/sys/security/audit/audit_bsm_klib.c ============================================================================== --- head/sys/security/audit/audit_bsm_klib.c Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/security/audit/audit_bsm_klib.c Fri Nov 30 23:18:49 2012 (r243726) @@ -462,13 +462,13 @@ auditon_command_event(int cmd) * leave the filename starting with '/' in the audit log in this case. */ void -audit_canon_path(struct thread *td, char *path, char *cpath) +audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) { struct vnode *cvnp, *rvnp; char *rbuf, *fbuf, *copy; struct filedesc *fdp; struct sbuf sbf; - int error, cwir; + int error, needslash, vfslocked; WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "%s: at %s:%d", __func__, __FILE__, __LINE__); @@ -491,10 +491,27 @@ audit_canon_path(struct thread *td, char * path. */ if (*path != '/') { - cvnp = fdp->fd_cdir; - vhold(cvnp); + if (dirfd == AT_FDCWD) { + cvnp = fdp->fd_cdir; + vhold(cvnp); + } else { + /* XXX: fgetvp() that vhold()s vnode instead of vref()ing it would be better */ + error = fgetvp(td, dirfd, 0, &cvnp); + if (error) { + cpath[0] = '\0'; + if (rvnp != NULL) + vdrop(rvnp); + return; + } + vhold(cvnp); + vfslocked = VFS_LOCK_GIANT(cvnp->v_mount); + vrele(cvnp); + VFS_UNLOCK_GIANT(vfslocked); + } + needslash = (fdp->fd_rdir != cvnp); + } else { + needslash = 1; } - cwir = (fdp->fd_rdir == fdp->fd_cdir); FILEDESC_SUNLOCK(fdp); /* * NB: We require that the supplied array be at least MAXPATHLEN bytes @@ -536,7 +553,7 @@ audit_canon_path(struct thread *td, char (void) sbuf_cat(&sbf, rbuf); free(fbuf, M_TEMP); } - if (cwir == 0 || (cwir != 0 && cvnp == NULL)) + if (needslash) (void) sbuf_putc(&sbf, '/'); /* * Now that we have processed any alternate root and relative path Modified: head/sys/security/audit/audit_private.h ============================================================================== --- head/sys/security/audit/audit_private.h Fri Nov 30 23:13:56 2012 (r243725) +++ head/sys/security/audit/audit_private.h Fri Nov 30 23:18:49 2012 (r243726) @@ -388,7 +388,8 @@ au_event_t audit_flags_and_error_to_ope au_event_t audit_flags_and_error_to_openatevent(int oflags, int error); au_event_t audit_msgctl_to_event(int cmd); au_event_t audit_semctl_to_event(int cmr); -void audit_canon_path(struct thread *td, char *path, char *cpath); +void audit_canon_path(struct thread *td, int dirfd, char *path, + char *cpath); au_event_t auditon_command_event(int cmd); /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211302318.qAUNInIh074238>