From owner-freebsd-fs@freebsd.org Wed Jul 8 22:07:24 2015 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2D05B996548 for ; Wed, 8 Jul 2015 22:07:24 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wi0-x229.google.com (mail-wi0-x229.google.com [IPv6:2a00:1450:400c:c05::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id AD44130EC; Wed, 8 Jul 2015 22:07:23 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by wiclp1 with SMTP id lp1so93150230wic.0; Wed, 08 Jul 2015 15:07:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=D4zuIoia69j1TtLKGQrffm2srmPYfbFbX+THxuMbwv0=; b=nTweBj1PSMgsZy9uc3FpSaRVF1KOuSkhYZvsJCoppidcfySHjAkn+GK5RQKqyYZ+cm wv7GKfXEON7/Iy303Dc5WeOnlCuAqYd/IdZAmPlCTHELHql1xwCYp9UQiwH4flvunppu 7dgbtfyt7gMSUyNG+URqZYX8t/aDk639TMuJ38IVN+2OhzBSHhRhNViMWIhmKzg7Piym i1s1Y6J/H/BEySmlFGNU7Ufhs2M2recwpMiOeP/iW7mu8vz0A7ntJCBhRZv0QI1ulwG8 KDvEEWFDU7gJkMi6vMbsgK/UBeeIeQ9dn8Zbq2+U4+Wql5ScXbURht6EtCOi1u9t5d9F lJbw== X-Received: by 10.194.192.72 with SMTP id he8mr24125279wjc.11.1436393242272; Wed, 08 Jul 2015 15:07:22 -0700 (PDT) Received: from localhost.localdomain (ip-89-102-11-63.net.upcbroadband.cz. [89.102.11.63]) by smtp.gmail.com with ESMTPSA id fo17sm5483921wjc.46.2015.07.08.15.07.20 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 08 Jul 2015 15:07:21 -0700 (PDT) From: Mateusz Guzik To: Konstantin Belousov Cc: rwatson@FreeBSD.org, freebsd-fs@freebsd.org, Mateusz Guzik Subject: [PATCH 4/4] audit: utilize vnode pointer found by namei instead of looking it up again Date: Thu, 9 Jul 2015 00:07:11 +0200 Message-Id: <1436393231-5831-5-git-send-email-mjguzik@gmail.com> X-Mailer: git-send-email 2.4.3 In-Reply-To: <1436393231-5831-1-git-send-email-mjguzik@gmail.com> References: <20150707085857.GZ2080@kib.kiev.ua> <1436393231-5831-1-git-send-email-mjguzik@gmail.com> X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jul 2015 22:07:24 -0000 From: Mateusz Guzik With the file descriptor translated only once, the code no longer longer imposes the need to hold filedesc lock, previously needed to make sure namei and audit translation return the same vnode. --- sys/kern/vfs_lookup.c | 21 ++++++---- sys/security/audit/audit.h | 14 +++++++ sys/security/audit/audit_arg.c | 36 ++++++++++++++++ sys/security/audit/audit_bsm_klib.c | 82 ++++++++++++++++++++++++------------- sys/security/audit/audit_private.h | 2 + 5 files changed, 118 insertions(+), 37 deletions(-) diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c index d48fcff..48d92e6 100644 --- a/sys/kern/vfs_lookup.c +++ b/sys/kern/vfs_lookup.c @@ -237,14 +237,6 @@ namei(struct nameidata *ndp) VREF(ndp->ni_rootdir); 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); - startdir_used = 0; dp = NULL; cnp->cn_nameptr = cnp->cn_pnbuf; @@ -268,8 +260,12 @@ namei(struct nameidata *ndp) if (cnp->cn_flags & AUDITVNODE2) AUDIT_ARG_ATFD2(ndp->ni_dirfd); error = fgetvp_rights(td, ndp->ni_dirfd, - &rights, &ndp->ni_filecaps, &dp); + NULL, &ndp->ni_filecaps, &dp); #ifdef CAPABILITIES + if (error == 0) { + error = cap_check(&ndp->ni_filecaps.fc_rights, + &rights); + } /* * If file descriptor doesn't have all rights, * all lookups relative to it must also be @@ -287,6 +283,13 @@ namei(struct nameidata *ndp) if (error == 0 && dp->v_type != VDIR) error = ENOTDIR; } + /* + * If we are auditing the kernel pathname, save the user pathname. + */ + if (cnp->cn_flags & AUDITVNODE1) + AUDIT_ARG_UPATH1_VP(td, dp, cnp->cn_pnbuf); + if (cnp->cn_flags & AUDITVNODE2) + AUDIT_ARG_UPATH2_VP(td, dp, cnp->cn_pnbuf); FILEDESC_SUNLOCK(fdp); if (ndp->ni_startdir != NULL && !startdir_used) vrele(ndp->ni_startdir); diff --git a/sys/security/audit/audit.h b/sys/security/audit/audit.h index 559d571..2f6420f 100644 --- a/sys/security/audit/audit.h +++ b/sys/security/audit/audit.h @@ -101,6 +101,10 @@ 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, int dirfd, char *upath); void audit_arg_upath2(struct thread *td, int dirfd, char *upath); +void audit_arg_upath1_vp(struct thread *td, struct vnode *dirvp, + char *upath); +void audit_arg_upath2_vp(struct thread *td, struct vnode *dirvp, + char *upath); void audit_arg_vnode1(struct vnode *vp); void audit_arg_vnode2(struct vnode *vp); void audit_arg_text(char *text); @@ -297,6 +301,16 @@ void audit_thread_free(struct thread *td); audit_arg_upath2((td), (dirfd), (upath)); \ } while (0) +#define AUDIT_ARG_UPATH1_VP(td, dirvp, upath) do { \ + if (AUDITING_TD(curthread)) \ + audit_arg_upath1_vp((td), (dirvp), (upath)); \ +} while (0) + +#define AUDIT_ARG_UPATH2_VP(td, dirvp, upath) do { \ + if (AUDITING_TD(curthread)) \ + audit_arg_upath2_vp((td), (dirvp), (upath)); \ +} while (0) + #define AUDIT_ARG_VALUE(value) do { \ if (AUDITING_TD(curthread)) \ audit_arg_value((value)); \ diff --git a/sys/security/audit/audit_arg.c b/sys/security/audit/audit_arg.c index c006b90..c019bad 100644 --- a/sys/security/audit/audit_arg.c +++ b/sys/security/audit/audit_arg.c @@ -719,6 +719,16 @@ audit_arg_upath(struct thread *td, int dirfd, char *upath, char **pathp) audit_canon_path(td, dirfd, upath, *pathp); } +static void +audit_arg_upath_vp(struct thread *td, struct vnode *dirvp, char *upath, + char **pathp) +{ + + if (*pathp == NULL) + *pathp = malloc(MAXPATHLEN, M_AUDITPATH, M_WAITOK); + audit_canon_path_vp(td, dirvp, upath, *pathp); +} + void audit_arg_upath1(struct thread *td, int dirfd, char *upath) { @@ -745,6 +755,32 @@ audit_arg_upath2(struct thread *td, int dirfd, char *upath) ARG_SET_VALID(ar, ARG_UPATH2); } +void +audit_arg_upath1_vp(struct thread *td, struct vnode *dirvp, char *upath) +{ + struct kaudit_record *ar; + + ar = currecord(); + if (ar == NULL) + return; + + audit_arg_upath_vp(td, dirvp, upath, &ar->k_ar.ar_arg_upath1); + ARG_SET_VALID(ar, ARG_UPATH1); +} + +void +audit_arg_upath2_vp(struct thread *td, struct vnode *dirvp, char *upath) +{ + struct kaudit_record *ar; + + ar = currecord(); + if (ar == NULL) + return; + + audit_arg_upath_vp(td, dirvp, upath, &ar->k_ar.ar_arg_upath2); + ARG_SET_VALID(ar, ARG_UPATH2); +} + /* * Function to save the path and vnode attr information into the audit * record. diff --git a/sys/security/audit/audit_bsm_klib.c b/sys/security/audit/audit_bsm_klib.c index b687a15..7e8dac5 100644 --- a/sys/security/audit/audit_bsm_klib.c +++ b/sys/security/audit/audit_bsm_klib.c @@ -461,23 +461,19 @@ auditon_command_event(int cmd) * but this results in a volfs name written to the audit log. So we will * leave the filename starting with '/' in the audit log in this case. */ -void -audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) +static void +audit_canon_path_common(struct thread *td, struct vnode *dirvp, + char *path, char *cpath) { struct vnode *cvnp, *rvnp; - char *rbuf, *fbuf, *copy; struct filedesc *fdp; + char *rbuf, *fbuf, *copy; struct sbuf sbf; - cap_rights_t rights; int error, needslash; - WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "%s: at %s:%d", - __func__, __FILE__, __LINE__); - copy = path; - rvnp = cvnp = NULL; + cvnp = rvnp = NULL; fdp = td->td_proc->p_fd; - FILEDESC_SLOCK(fdp); /* * Make sure that we handle the chroot(2) case. If there is an * alternate root directory, prepend it to the audited pathname. @@ -492,22 +488,7 @@ audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) * path. */ if (*path != '/') { - 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, cap_rights_init(&rights), &cvnp); - if (error) { - FILEDESC_SUNLOCK(fdp); - cpath[0] = '\0'; - if (rvnp != NULL) - vdrop(rvnp); - return; - } - vhold(cvnp); - vrele(cvnp); - } + cvnp = dirvp; needslash = (fdp->fd_rdir != cvnp); } else { needslash = 1; @@ -536,8 +517,6 @@ audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) vdrop(rvnp); if (error) { cpath[0] = '\0'; - if (cvnp != NULL) - vdrop(cvnp); return; } (void) sbuf_cat(&sbf, rbuf); @@ -545,7 +524,6 @@ audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) } if (cvnp != NULL) { error = vn_fullpath(td, cvnp, &rbuf, &fbuf); - vdrop(cvnp); if (error) { cpath[0] = '\0'; return; @@ -571,3 +549,51 @@ audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) } sbuf_finish(&sbf); } + +void +audit_canon_path(struct thread *td, int dirfd, char *path, char *cpath) +{ + struct vnode *dirvp; + struct filedesc *fdp; + cap_rights_t rights; + + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "%s: at %s:%d", + __func__, __FILE__, __LINE__); + + dirvp = NULL; + fdp = td->td_proc->p_fd; + FILEDESC_SLOCK(fdp); + if (*path != '/') { + if (dirfd == AT_FDCWD) { + dirvp = fdp->fd_cdir; + vhold(dirvp); + } else { + /* XXX: fgetvp() that vhold()s vnode instead of vref()ing it would be better */ + if (fgetvp(td, dirfd, cap_rights_init(&rights), &dirvp) != 0) { + FILEDESC_SUNLOCK(fdp); + cpath[0] = '\0'; + return; + } + vhold(dirvp); + vrele(dirvp); + } + } + + audit_canon_path_common(td, dirvp, path, cpath); + if (dirvp != NULL) + vdrop(dirvp); +} + +void +audit_canon_path_vp(struct thread *td, struct vnode *dirvp, char *path, char *cpath) +{ + + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "%s: at %s:%d", + __func__, __FILE__, __LINE__); + + if (dirvp == NULL) + return; + + FILEDESC_SLOCK(td->td_proc->p_fd); + audit_canon_path_common(td, dirvp, path, cpath); +} diff --git a/sys/security/audit/audit_private.h b/sys/security/audit/audit_private.h index b5c373a..7ecf3a6 100644 --- a/sys/security/audit/audit_private.h +++ b/sys/security/audit/audit_private.h @@ -394,6 +394,8 @@ 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, int dirfd, char *path, char *cpath); +void audit_canon_path_vp(struct thread *td, struct vnode *dirvp, + char *path, char *cpath); au_event_t auditon_command_event(int cmd); /* -- 2.4.5