public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
From: duchangbin <changbin.du@huawei.com>
To: Jonathan Corbet <corbet@lwn.net>
Cc: duchangbin <changbin.du@huawei.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tools: jobserver: Add validation for jobserver tokens to ensure valid '+' characters
Date: Wed, 7 Jan 2026 08:11:29 +0000	[thread overview]
Message-ID: <bc19bb55bee34abb990c00c3006c6710@huawei.com> (raw)
In-Reply-To: <87zf6qcsu1.fsf@trenco.lwn.net>

On Tue, Jan 06, 2026 at 02:52:06PM -0700, Jonathan Corbet wrote:
> Changbin Du <changbin.du@huawei.com> writes:
> 
> > Add validation for jobserver tokens to prevent infinite loops on invalid fds
> > When using GNU Make's jobserver feature in kernel builds, a bug in MAKEFLAGS
> > propagation caused "--jobserver-auth=3,4" to reference an unintended file
> > descriptor (Here, fd 3 was inherited from a shell command that opened
> > "/etc/passwd" instead of a valid pipe). This led to infinite loops in
> > jobserver-exec's os.read() calls due to empty or corrupted tokens. (The
> > version of my make is 4.3)
> >
> > $ ls -l /proc/self/fd
> > total 0
> > lrwx------ 1 changbin changbin 64 Dec 25 13:03 0 -> /dev/pts/1
> > lrwx------ 1 changbin changbin 64 Dec 25 13:03 1 -> /dev/pts/1
> > lrwx------ 1 changbin changbin 64 Dec 25 13:03 2 -> /dev/pts/1
> > lr-x------ 1 changbin changbin 64 Dec 25 13:03 3 -> /etc/passwd
> > lr-x------ 1 changbin changbin 64 Dec 25 13:03 4 -> /proc/1421383/fd
> >
> > The modified code now explicitly validates tokens:
> > 1. Rejects empty reads (prevents infinite loops on EOF)
> > 2. Checks all bytes are '+' characters (catches fd reuse issues)
> > 3. Raises ValueError with clear diagnostics for debugging
> > This ensures robustness against invalid jobserver configurations, even when
> > external tools (like make) incorrectly pass non-pipe file descriptors.
> >
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> >  tools/lib/python/jobserver.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/lib/python/jobserver.py b/tools/lib/python/jobserver.py
> > index a24f30ef4fa8..88d005f96bed 100755
> > --- a/tools/lib/python/jobserver.py
> > +++ b/tools/lib/python/jobserver.py
> > @@ -91,6 +91,8 @@ class JobserverExec:
> >              while True:
> >                  try:
> >                      slot = os.read(self.reader, 8)
> > +                    if not slot or any(c != b'+'[0] for c in slot):
> > +                        raise ValueError("empty or unexpected token from jobserver")
> 
> So I had to stare at this for a while to figure out what it was doing; a
> comment might help.
> 
> But if it finds something that's not b'+', it simply crashes the whole
> thing?  Is that really what we want to do?  It would seem better to
> proceed if we got any slots at all, and to emit a message telling the
> poor user what they might want to do about the situation?
>
I suspect that in Make versions prior to 4.3, when generating the "--jobserver-auth=r,w"
parameter, the implementation fails to properly handle situations where file descriptor 3
is already occupied by the parent process (as I encountered where fd 3 was actually used to
open /etc/passwd). This appears to force Make to always use fd3 regardless of its
availability (I'm not sure how Make was written). In contrast, Make 4.4+ versions
default to using named pipes, which avoids this issue entirely.

When this problem occurs, the current implementation deadlocks because for regular files,
os.read() returns empty bytes after reaching EOF, creating an infinite loop. My workaround
is to ignore this error condition to prevent deadlock, although this means the jobserver
protocol will no longer be honored.

As you suggested above, We can output an error message to stderr to inform users, but
must not use stdout, as it would corrupt the tool's normal output stream. For
example, in scripts/Makefile.vmlinux_o we have:

quiet_cmd_gen_initcalls_lds = GEN     $@
      cmd_gen_initcalls_lds = \
        $(PYTHON3) $(srctree)/scripts/jobserver-exec \
        $(PERL) $(real-prereqs) > $@


> >                      self.jobs += slot
> >                  except (OSError, IOError) as e:
> >                      if e.errno == errno.EWOULDBLOCK:
> 
> Thanks,
> 
> jon

-- 
Cheers,
Changbin Du

  reply	other threads:[~2026-01-07  8:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-25  6:26 [PATCH] tools: jobserver: Add validation for jobserver tokens to ensure valid '+' characters Changbin Du
2026-01-05  8:22 ` duchangbin
2026-01-05 15:35   ` Jonathan Corbet
2026-01-06 21:52 ` Jonathan Corbet
2026-01-07  8:11   ` duchangbin [this message]
2026-01-07  9:29     ` Mauro Carvalho Chehab
2026-01-07 10:42       ` Mauro Carvalho Chehab
2026-01-07 10:54         ` Mauro Carvalho Chehab
2026-01-08  2:58         ` duchangbin
2026-01-08  8:24           ` Mauro Carvalho Chehab
2026-01-08 10:01             ` duchangbin

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=bc19bb55bee34abb990c00c3006c6710@huawei.com \
    --to=changbin.du@huawei.com \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@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