From owner-svn-src-head@freebsd.org Thu Nov 19 06:30:27 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1E3FD2E91DB; Thu, 19 Nov 2020 06:30:27 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cc8tq0Jr1z3mp4; Thu, 19 Nov 2020 06:30:27 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id ED8061F2FA; Thu, 19 Nov 2020 06:30:26 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0AJ6UQoC004829; Thu, 19 Nov 2020 06:30:26 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0AJ6UPvR004824; Thu, 19 Nov 2020 06:30:25 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202011190630.0AJ6UPvR004824@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Thu, 19 Nov 2020 06:30:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r367833 - in head/sys: kern security/mac sys X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: in head/sys: kern security/mac sys X-SVN-Commit-Revision: 367833 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2020 06:30:27 -0000 Author: mjg Date: Thu Nov 19 06:30:25 2020 New Revision: 367833 URL: https://svnweb.freebsd.org/changeset/base/367833 Log: pipe: allow for lockless pipe_stat pipes get stated all thet time and this avoidably contributed to contention. The pipe lock is only held to accomodate MAC and to check the type. Since normally there is no probe for pipe stat depessimize this by having the flag. The pipe_state field gets modified with locks held all the time and it's not feasible to convert them to use atomic store. Move the type flag away to a separate variable as a simple cleanup and to provide stable field to read. Use short for both fields to avoid growing the struct. While here short-circuit MAC for pipe_poll as well. Modified: head/sys/kern/sys_pipe.c head/sys/security/mac/mac_framework.c head/sys/security/mac/mac_framework.h head/sys/security/mac/mac_pipe.c head/sys/sys/pipe.h Modified: head/sys/kern/sys_pipe.c ============================================================================== --- head/sys/kern/sys_pipe.c Thu Nov 19 05:46:59 2020 (r367832) +++ head/sys/kern/sys_pipe.c Thu Nov 19 06:30:25 2020 (r367833) @@ -140,7 +140,7 @@ __FBSDID("$FreeBSD$"); /* #define PIPE_NODIRECT */ #define PIPE_PEER(pipe) \ - (((pipe)->pipe_state & PIPE_NAMED) ? (pipe) : ((pipe)->pipe_peer)) + (((pipe)->pipe_type & PIPE_TYPE_NAMED) ? (pipe) : ((pipe)->pipe_peer)) /* * interfaces to the outside world @@ -403,7 +403,7 @@ pipe_named_ctor(struct pipe **ppipe, struct thread *td error = pipe_paircreate(td, &pp); if (error != 0) return (error); - pp->pp_rpipe.pipe_state |= PIPE_NAMED; + pp->pp_rpipe.pipe_type |= PIPE_TYPE_NAMED; *ppipe = &pp->pp_rpipe; return (0); } @@ -413,7 +413,7 @@ pipe_dtor(struct pipe *dpipe) { struct pipe *peer; - peer = (dpipe->pipe_state & PIPE_NAMED) != 0 ? dpipe->pipe_peer : NULL; + peer = (dpipe->pipe_type & PIPE_TYPE_NAMED) != 0 ? dpipe->pipe_peer : NULL; funsetown(&dpipe->pipe_sigio); pipeclose(dpipe); if (peer != NULL) { @@ -1328,7 +1328,7 @@ pipe_truncate(struct file *fp, off_t length, struct uc int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vnops.fo_truncate(fp, length, active_cred, td); else error = invfo_truncate(fp, length, active_cred, td); @@ -1443,7 +1443,7 @@ pipe_poll(struct file *fp, int events, struct ucred *a levents = events & (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | POLLRDBAND); - if (rpipe->pipe_state & PIPE_NAMED && fp->f_flag & FREAD && levents && + if (rpipe->pipe_type & PIPE_TYPE_NAMED && fp->f_flag & FREAD && levents && fp->f_pipegen == rpipe->pipe_wgen) events |= POLLINIGNEOF; @@ -1496,23 +1496,22 @@ pipe_stat(struct file *fp, struct stat *ub, struct ucr #endif pipe = fp->f_data; - PIPE_LOCK(pipe); #ifdef MAC - error = mac_pipe_check_stat(active_cred, pipe->pipe_pair); - if (error) { + if (mac_pipe_check_stat_enabled()) { + PIPE_LOCK(pipe); + error = mac_pipe_check_stat(active_cred, pipe->pipe_pair); PIPE_UNLOCK(pipe); - return (error); + if (error) { + return (error); + } } #endif /* For named pipes ask the underlying filesystem. */ - if (pipe->pipe_state & PIPE_NAMED) { - PIPE_UNLOCK(pipe); + if (pipe->pipe_type & PIPE_TYPE_NAMED) { return (vnops.fo_stat(fp, ub, active_cred, td)); } - PIPE_UNLOCK(pipe); - bzero(ub, sizeof(*ub)); ub->st_mode = S_IFIFO; ub->st_blksize = PAGE_SIZE; @@ -1554,7 +1553,7 @@ pipe_chmod(struct file *fp, mode_t mode, struct ucred int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vn_chmod(fp, mode, active_cred, td); else error = invfo_chmod(fp, mode, active_cred, td); @@ -1569,7 +1568,7 @@ pipe_chown(struct file *fp, uid_t uid, gid_t gid, stru int error; cpipe = fp->f_data; - if (cpipe->pipe_state & PIPE_NAMED) + if (cpipe->pipe_type & PIPE_TYPE_NAMED) error = vn_chown(fp, uid, gid, active_cred, td); else error = invfo_chown(fp, uid, gid, active_cred, td); @@ -1758,7 +1757,7 @@ filt_piperead(struct knote *kn, long hint) kn->kn_data = rpipe->pipe_pages.cnt; if ((rpipe->pipe_state & PIPE_EOF) != 0 && - ((rpipe->pipe_state & PIPE_NAMED) == 0 || + ((rpipe->pipe_type & PIPE_TYPE_NAMED) == 0 || fp->f_pipegen != rpipe->pipe_wgen)) { kn->kn_flags |= EV_EOF; return (1); @@ -1778,7 +1777,7 @@ filt_pipewrite(struct knote *kn, long hint) * knlist and the list lock (i.e., the pipe lock) is therefore not held. */ if (wpipe->pipe_present == PIPE_ACTIVE || - (wpipe->pipe_state & PIPE_NAMED) != 0) { + (wpipe->pipe_type & PIPE_TYPE_NAMED) != 0) { PIPE_LOCK_ASSERT(wpipe, MA_OWNED); if (wpipe->pipe_state & PIPE_DIRECTW) { Modified: head/sys/security/mac/mac_framework.c ============================================================================== --- head/sys/security/mac/mac_framework.c Thu Nov 19 05:46:59 2020 (r367832) +++ head/sys/security/mac/mac_framework.c Thu Nov 19 06:30:25 2020 (r367833) @@ -141,6 +141,8 @@ FPFLAG(vnode_check_mmap); FPFLAG_RARE(vnode_check_poll); FPFLAG_RARE(vnode_check_rename_from); FPFLAG_RARE(vnode_check_access); +FPFLAG_RARE(pipe_check_stat); +FPFLAG_RARE(pipe_check_poll); #undef FPFLAG #undef FPFLAG_RARE @@ -433,6 +435,10 @@ struct mac_policy_fastpath_elem mac_policy_fastpath_ar .flag = &mac_vnode_check_rename_from_fp_flag }, { .offset = FPO(vnode_check_access), .flag = &mac_vnode_check_access_fp_flag }, + { .offset = FPO(pipe_check_stat), + .flag = &mac_pipe_check_stat_fp_flag }, + { .offset = FPO(pipe_check_poll), + .flag = &mac_pipe_check_poll_fp_flag }, }; static void Modified: head/sys/security/mac/mac_framework.h ============================================================================== --- head/sys/security/mac/mac_framework.h Thu Nov 19 05:46:59 2020 (r367832) +++ head/sys/security/mac/mac_framework.h Thu Nov 19 06:30:25 2020 (r367833) @@ -208,9 +208,30 @@ void mac_netinet6_nd6_send(struct ifnet *ifp, struct m int mac_pipe_check_ioctl(struct ucred *cred, struct pipepair *pp, unsigned long cmd, void *data); -int mac_pipe_check_poll(struct ucred *cred, struct pipepair *pp); -int mac_pipe_check_read(struct ucred *cred, struct pipepair *pp); +int mac_pipe_check_poll_impl(struct ucred *cred, struct pipepair *pp); +#ifdef MAC +extern bool mac_pipe_check_poll_fp_flag; +#else +#define mac_pipe_check_poll_fp_flag 0 +#endif +#define mac_pipe_check_poll_enabled() __predict_false(mac_pipe_check_poll_fp_flag) +static inline int +mac_pipe_check_poll(struct ucred *cred, struct pipepair *pp) +{ + + if (mac_pipe_check_poll_enabled()) + return (mac_pipe_check_poll_impl(cred, pp)); + return (0); +} + +#ifdef MAC +extern bool mac_pipe_check_stat_fp_flag; +#else +#define mac_pipe_check_stat_fp_flag 0 +#endif +#define mac_pipe_check_stat_enabled() __predict_false(mac_pipe_check_stat_fp_flag) int mac_pipe_check_stat(struct ucred *cred, struct pipepair *pp); +int mac_pipe_check_read(struct ucred *cred, struct pipepair *pp); int mac_pipe_check_write(struct ucred *cred, struct pipepair *pp); void mac_pipe_create(struct ucred *cred, struct pipepair *pp); void mac_pipe_destroy(struct pipepair *); Modified: head/sys/security/mac/mac_pipe.c ============================================================================== --- head/sys/security/mac/mac_pipe.c Thu Nov 19 05:46:59 2020 (r367832) +++ head/sys/security/mac/mac_pipe.c Thu Nov 19 06:30:25 2020 (r367833) @@ -163,7 +163,7 @@ MAC_CHECK_PROBE_DEFINE2(pipe_check_poll, "struct ucred "struct pipepair *"); int -mac_pipe_check_poll(struct ucred *cred, struct pipepair *pp) +mac_pipe_check_poll_impl(struct ucred *cred, struct pipepair *pp) { int error; Modified: head/sys/sys/pipe.h ============================================================================== --- head/sys/sys/pipe.h Thu Nov 19 05:46:59 2020 (r367832) +++ head/sys/sys/pipe.h Thu Nov 19 06:30:25 2020 (r367833) @@ -95,9 +95,13 @@ struct pipemapping { #define PIPE_LWANT 0x200 /* Process wants exclusive access to pointers/data. */ #define PIPE_DIRECTW 0x400 /* Pipe direct write active. */ #define PIPE_DIRECTOK 0x800 /* Direct mode ok. */ -#define PIPE_NAMED 0x1000 /* Is a named pipe. */ /* + * Bits in pipe_type. + */ +#define PIPE_TYPE_NAMED 0x001 /* Is a named pipe. */ + +/* * Per-pipe data structure. * Two of these are linked together to produce bi-directional pipes. */ @@ -111,7 +115,8 @@ struct pipe { struct sigio *pipe_sigio; /* information for async I/O */ struct pipe *pipe_peer; /* link with other direction */ struct pipepair *pipe_pair; /* container structure pointer */ - u_int pipe_state; /* pipe status info */ + u_short pipe_state; /* pipe status info */ + u_short pipe_type; /* pipe type info */ int pipe_busy; /* busy flag, mostly to handle rundown sanely */ int pipe_present; /* still present? */ int pipe_wgen; /* writer generation for named pipe */