From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Kees Cook <keescook@chromium.org>, Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] docs, parallelism: Do not leak blocking mode to writer
Date: Thu, 21 Nov 2019 08:41:01 +0100 [thread overview]
Message-ID: <041953ef-0b6c-4ea8-8734-aa1e6703f9f8@rasmusvillemoes.dk> (raw)
In-Reply-To: <20191121000304.48829-3-keescook@chromium.org>
On 21/11/2019 01.03, Kees Cook wrote:
> Setting non-blocking via a local copy of the jobserver file descriptor
> is safer than just assuming the writer on the original fd is prepared
> for it to be non-blocking.
This is a bit inaccurate. The fd referring to the write side of the pipe
is always blocking - it has to be, due to the protocol requiring you to
write back the tokens you've read, so you can't just drop a token on the
floor. But it's also rather moot, since the pipe will never hold
anywhere near 4096 bytes, let alone a (linux) pipe's default capacity of
64K.
But what we cannot do is change the mode of the open file description to
non-blocking for the read side, in case the parent make (or some sibling
process that has also inherited the same "struct file") expects it to be
blocking.
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Link: https://lore.kernel.org/lkml/44c01043-ab24-b4de-6544-e8efd153e27a@rasmusvillemoes.dk
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> scripts/jobserver-count | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/jobserver-count b/scripts/jobserver-count
> index 6e15b38df3d0..a68a04ad304f 100755
> --- a/scripts/jobserver-count
> +++ b/scripts/jobserver-count
> @@ -12,12 +12,6 @@ default="1"
> if len(sys.argv) > 1:
> default=sys.argv[1]
>
> -# Set non-blocking for a given file descriptor.
> -def nonblock(fd):
> - flags = fcntl.fcntl(fd, fcntl.F_GETFL)
> - fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
> - return fd
> -
> # Extract and prepare jobserver file descriptors from envirnoment.
> try:
> # Fetch the make environment options.
> @@ -31,8 +25,13 @@ try:
> # Parse out R,W file descriptor numbers and set them nonblocking.
> fds = opts[0].split("=", 1)[1]
> reader, writer = [int(x) for x in fds.split(",", 1)]
> - reader = nonblock(reader)
> -except (KeyError, IndexError, ValueError, IOError):
> + # Open a private copy of reader to avoid setting nonblocking
> + # on an unexpecting writer.
s/writer/reader/
> + reader = os.open("/proc/self/fd/%d" % (reader), os.O_RDONLY)
> + flags = fcntl.fcntl(reader, fcntl.F_GETFL)
> + fcntl.fcntl(reader, fcntl.F_SETFL, flags | os.O_NONBLOCK)
I think you can just specify O_NONBLOCK in the open() call so you avoid
those two fcntls.
Rasmus
next prev parent reply other threads:[~2019-11-21 7:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-21 0:03 [PATCH 0/3] docs, parallelism: Rearrange how jobserver reservations are made Kees Cook
2019-11-21 0:03 ` [PATCH 1/3] docs, parallelism: Fix failure path and add comment Kees Cook
2019-11-21 0:03 ` [PATCH 2/3] docs, parallelism: Do not leak blocking mode to writer Kees Cook
2019-11-21 7:41 ` Rasmus Villemoes [this message]
2019-11-21 19:09 ` Kees Cook
2019-11-21 0:03 ` [PATCH 3/3] docs, parallelism: Rearrange how jobserver reservations are made Kees Cook
2019-11-21 8:09 ` Rasmus Villemoes
2019-11-21 19:39 ` Kees Cook
2019-11-21 19:52 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=041953ef-0b6c-4ea8-8734-aa1e6703f9f8@rasmusvillemoes.dk \
--to=linux@rasmusvillemoes.dk \
--cc=corbet@lwn.net \
--cc=keescook@chromium.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).