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@xxxxxxxxxxxxxxxxxx> > Link: https://lore.kernel.org/lkml/44c01043-ab24-b4de-6544-e8efd153e27a@xxxxxxxxxxxxxxxxxx > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > --- > 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
