From owner-freebsd-hackers@FreeBSD.ORG Wed Apr 9 11:26:37 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 4E235908 for ; Wed, 9 Apr 2014 11:26:37 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id CA244191D for ; Wed, 9 Apr 2014 11:26:36 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.8/8.14.8) with ESMTP id s39BQRdF060806; Wed, 9 Apr 2014 14:26:27 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua s39BQRdF060806 Received: (from kostik@localhost) by tom.home (8.14.8/8.14.8/Submit) id s39BQRow060805; Wed, 9 Apr 2014 14:26:27 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 9 Apr 2014 14:26:27 +0300 From: Konstantin Belousov To: Mateusz Guzik Subject: Re: pipe() resource exhaustion Message-ID: <20140409112627.GI21331@kib.kiev.ua> References: <20140408130206.e75f3bf6c6df28b6e4839e70@yahoo.es> <20140408121222.GB30326@dft-labs.eu> <20140408123827.GW21331@kib.kiev.ua> <20140408130727.GA11363@dft-labs.eu> <20140408132442.GZ21331@kib.kiev.ua> <20140409111654.GA17650@dft-labs.eu> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R7RSAOS08SfIpCMC" Content-Disposition: inline In-Reply-To: <20140409111654.GA17650@dft-labs.eu> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: freebsd-hackers@freebsd.org, Eduardo Morras X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Apr 2014 11:26:37 -0000 --R7RSAOS08SfIpCMC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 09, 2014 at 01:16:54PM +0200, Mateusz Guzik wrote: > That said how about the following: Looks fine to me, with one note about the comment. See below. >=20 > diff --git a/sys/fs/fifofs/fifo_vnops.c b/sys/fs/fifofs/fifo_vnops.c > index d3eb281..a3e8179 100644 > --- a/sys/fs/fifofs/fifo_vnops.c > +++ b/sys/fs/fifofs/fifo_vnops.c > @@ -146,9 +146,7 @@ fifo_open(ap) > if (fp =3D=3D NULL || (ap->a_mode & FEXEC) !=3D 0) > return (EINVAL); > if ((fip =3D vp->v_fifoinfo) =3D=3D NULL) { > - error =3D pipe_named_ctor(&fpipe, td); > - if (error !=3D 0) > - return (error); > + pipe_named_ctor(&fpipe, td); > fip =3D malloc(sizeof(*fip), M_VNODE, M_WAITOK); > fip->fi_pipe =3D fpipe; > fpipe->pipe_wgen =3D fip->fi_readers =3D fip->fi_writers =3D 0; > diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c > index 6ba52e3..b0a1f0d 100644 > --- a/sys/kern/sys_pipe.c > +++ b/sys/kern/sys_pipe.c > @@ -221,8 +221,8 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, piperesizeallowed, CT= LFLAG_RW, > static void pipeinit(void *dummy __unused); > static void pipeclose(struct pipe *cpipe); > static void pipe_free_kmem(struct pipe *cpipe); > -static int pipe_create(struct pipe *pipe, int backing); > -static int pipe_paircreate(struct thread *td, struct pipepair **p_pp); > +static void pipe_create(struct pipe *pipe, int backing); > +static void pipe_paircreate(struct thread *td, struct pipepair **p_pp); > static __inline int pipelock(struct pipe *cpipe, int catch); > static __inline void pipeunlock(struct pipe *cpipe); > #ifndef PIPE_NODIRECT > @@ -331,12 +331,11 @@ pipe_zone_fini(void *mem, int size) > mtx_destroy(&pp->pp_mtx); > } > =20 > -static int > +static void > pipe_paircreate(struct thread *td, struct pipepair **p_pp) > { > struct pipepair *pp; > struct pipe *rpipe, *wpipe; > - int error; > =20 > *p_pp =3D pp =3D uma_zalloc(pipe_zone, M_WAITOK); > #ifdef MAC > @@ -355,30 +354,21 @@ pipe_paircreate(struct thread *td, struct pipepair = **p_pp) > knlist_init_mtx(&wpipe->pipe_sel.si_note, PIPE_MTX(wpipe)); > =20 > /* Only the forward direction pipe is backed by default */ > - if ((error =3D pipe_create(rpipe, 1)) !=3D 0 || > - (error =3D pipe_create(wpipe, 0)) !=3D 0) { > - pipeclose(rpipe); > - pipeclose(wpipe); > - return (error); > - } > + pipe_create(rpipe, 1); > + pipe_create(wpipe, 0); > =20 > rpipe->pipe_state |=3D PIPE_DIRECTOK; > wpipe->pipe_state |=3D PIPE_DIRECTOK; > - return (0); > } > =20 > -int > +void > pipe_named_ctor(struct pipe **ppipe, struct thread *td) > { > struct pipepair *pp; > - int error; > =20 > - error =3D pipe_paircreate(td, &pp); > - if (error !=3D 0) > - return (error); > + pipe_paircreate(td, &pp); > pp->pp_rpipe.pipe_state |=3D PIPE_NAMED; > *ppipe =3D &pp->pp_rpipe; > - return (0); > } > =20 > void > @@ -419,9 +409,7 @@ kern_pipe2(struct thread *td, int fildes[2], int flag= s) > int fd, fflags, error; > =20 > fdp =3D td->td_proc->p_fd; > - error =3D pipe_paircreate(td, &pp); > - if (error !=3D 0) > - return (error); > + pipe_paircreate(td, &pp); > rpipe =3D &pp->pp_rpipe; > wpipe =3D &pp->pp_wpipe; > error =3D falloc(td, &rf, &fd, flags); > @@ -642,24 +630,25 @@ pipeselwakeup(cpipe) > * Initialize and allocate VM and memory for pipe. The structure > * will start out zero'd from the ctor, so we just manage the kmem. > */ > -static int > +static void > pipe_create(pipe, backing) > struct pipe *pipe; > int backing; > { > - int error; > =20 > if (backing) { > + /* > + * Note that these functions can fail, but we ignore > + * the error as it is not fatal and could be provoked > + * by users. > + */ It would be benefitial to add some more details on the way to provoke the failure. Note in the comment that creating too much pipes would exhaust pipe map and we fall back to the buffer pipe creation there, which still work correctly, albeit slow. > if (amountpipekva > maxpipekva / 2) > - error =3D pipespace_new(pipe, SMALL_PIPE_SIZE); > + (void)pipespace_new(pipe, SMALL_PIPE_SIZE); > else > - error =3D pipespace_new(pipe, PIPE_SIZE); > - } else { > - /* If we're not backing this pipe, no need to do anything. */ > - error =3D 0; > + (void)pipespace_new(pipe, PIPE_SIZE); > } > + > pipe->pipe_ino =3D -1; > - return (error); > } > =20 > /* ARGSUSED */ > diff --git a/sys/sys/pipe.h b/sys/sys/pipe.h > index dfe7f2f..ee7c128 100644 > --- a/sys/sys/pipe.h > +++ b/sys/sys/pipe.h > @@ -142,6 +142,6 @@ struct pipepair { > #define PIPE_LOCK_ASSERT(pipe, type) mtx_assert(PIPE_MTX(pipe), (type)) > =20 > void pipe_dtor(struct pipe *dpipe); > -int pipe_named_ctor(struct pipe **ppipe, struct thread *td); > +void pipe_named_ctor(struct pipe **ppipe, struct thread *td); > void pipeselwakeup(struct pipe *cpipe); > #endif /* !_SYS_PIPE_H_ */ --R7RSAOS08SfIpCMC Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQIcBAEBAgAGBQJTRS5iAAoJEJDCuSvBvK1BQAYP/3o3nngqzNSaQGCvEcYc5DDr TXXy2d7dRWDYaBZM1q3SmyMSPxiDY9ZNpitbAwoy2a8VSARclS2mf4m6K+F01pxy 1Lbh7apAe2XBHRxb5jw4qsls0HQe8Y7bD3i528A0U/hJOKbc93IPfqwOrV9ij9FY d0ey6Z0OWbIPJhWwfC2u3fez7xhJBzPf+V1h0qdfHffxqbNBYySA8b6TL8qCynyD zWf+W5hk1kwd22vnOkcuxCflwHmz3yU5BgAxjhH++gwyyzA1CV3PVP8ajZSea4Zt T2gFfu9ghNDGeJYh3+nKi5x45ypMw6HcE13ExsvYzMo0mK3V1PtI2pxtd7l7sKKQ vRMYyL0oJ003ublkXaB7ViDlR4cyszTux3rdJqcwL+6tmFtrQDE/VUBMW+yxPV++ A7pjIoE70dlY5UqNDViF5BgjZ53/Ve6Ej4C0NQqNzy5XacdG9/gEvf2gK1IwmAl/ x/WUM9gKevSXYDhX7rDZiMaoZX9lNk0M03aAZ+QTc1weVEJkJQgktwo3vEZ7qs5y 6gcdZ0MLq7+w8EMz47E2jadetB/2yFhyN7PU6kmB5uNsP1aF5Q2GxdQznuGXQZzK g5PzL+IpX3SLcumnsL3OtOQyV/Vw3dEaet34htUkol4zl29CwHUz9zHnBtrhK5+G i2vjRvfqZ2P/mj91mh6S =F3VI -----END PGP SIGNATURE----- --R7RSAOS08SfIpCMC--