Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 8 Apr 2014 15:07:27 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        freebsd-hackers@freebsd.org, Eduardo Morras <emorrasg@yahoo.es>
Subject:   Re: pipe() resource exhaustion
Message-ID:  <20140408130727.GA11363@dft-labs.eu>
In-Reply-To: <20140408123827.GW21331@kib.kiev.ua>
References:  <lhu0jv$r6n$1@ger.gmane.org> <ab57e60fcc1c1438fcca500e3c594d35@mail.feld.me> <20140408130206.e75f3bf6c6df28b6e4839e70@yahoo.es> <20140408121222.GB30326@dft-labs.eu> <20140408123827.GW21331@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Apr 08, 2014 at 03:38:27PM +0300, Konstantin Belousov wrote:
> On Tue, Apr 08, 2014 at 02:12:22PM +0200, Mateusz Guzik wrote:
> > That said, supporting a reserve for this one sounds like a good idea and
> > not that hard to implement - one can either play with atomics and double
> > check or just place a mutex-protected check in pipespace_new (before
> > vm_map_find).
> > 
> ...
> 
> I think more reasonable behaviour there is to just fall back to the
> buffered pipe if the direct buffer allocation fails. Look at the
> pipespace_new() calls in the pipe_create(); probably ignoring the error
> would do the trick.

Yeah, should have checked the caller.

Interesting though how the error was made fatal in thiscase.

Anyhow, the following hack following your suggestion  indeed makes the
issue go away for me:

diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index 6ba52e3..5930cf2 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -647,19 +647,21 @@ pipe_create(pipe, backing)
 	struct pipe *pipe;
 	int backing;
 {
-	int error;
 
 	if (backing) {
+		/*
+		 * Note that these functions can fail, but we ignore
+		 * the error as it is not fatal and could be provoked
+		 * by users.
+		 */
 		if (amountpipekva > maxpipekva / 2)
-			error = pipespace_new(pipe, SMALL_PIPE_SIZE);
+			(void)pipespace_new(pipe, SMALL_PIPE_SIZE);
 		else
-			error = pipespace_new(pipe, PIPE_SIZE);
-	} else {
-		/* If we're not backing this pipe, no need to do anything. */
-		error = 0;
+			(void)pipespace_new(pipe, PIPE_SIZE);
 	}
+
 	pipe->pipe_ino = -1;
-	return (error);
+	return (0);
 }
 
 /* ARGSUSED */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140408130727.GA11363>