From: Jonathan de Boyne Pollard <J.deBoynePollard-newsgroups@NTLWorld.COM>
To: Supervision <supervision@list.skarnet.org>
Subject: Re: Handle ENOTDIR in pathexec_run
Date: Wed, 19 Feb 2020 08:18:31 +0000 [thread overview]
Message-ID: <5afb3f20-239b-f09a-c696-fe36d141b2cd@NTLWorld.COM> (raw)
In-Reply-To: <CADMWQoN4XCNAzm5weVd8kuBMMsZy+RP5KsFcBhAh1yqnmyjgRQ@mail.gmail.com>
Jacob Vosmaer:
> The patch below makes runit ignore this type of bad path entry.
> What do you think?
>
It's not good enough.
I was all set to apply this to djbwares, as the runit code for pathexec
is actually Daniel J. Bernstein's original code. But it highlighted
some basic problems of running execve() in a loop to do a PATH search.
Simply put: One wants to (largely) ignore the error if there's a problem
with the directory prefix from the search path; and one wants to retain
the error if there's a problem with the final pathname component, the
name that is being searched for or the actual node that it names.
Unfortunately, execve() can return EACCESS in both cases, and simply
deciding to exit the loop based upon nothing but the errno value is
therefore an unworkable strategy.
If you look at the BSD C library in FreeBSD, you'll see that it attempts
to distinguish between the two cases in its execvpe() by calling stat()
on error. If the stat() succeeds, then the problem cannot be with the
directory prefix. So it remembers the EACCESS, which must be a problem
with the file itself. If the stat() fails, then the problem is with the
directory prefix (or with symbolic link traversal, hence the use of
stat() not lstat()) and it largely ignores the EACCESS.
If you look at the internal_execve() function in the next release of the
nosh toolset, you'll see that I've used a different design and made use
of (relatively, they still being over a decade old) newer API functions,
instead. The code uses openat() on the path prefixes with O_DIRECTORY
(and O_CLOEXEC), if that fails largely ignoring that error and
continuing to the next PATH component. It then uses, relative to that
file descriptor, openat() with O_EXEC and then fexecve() for the final
component (which it also this way does not have to do string
concatenation with), retaining that error if that fails. Things are
slightly complicated by some trickiness relating to executable scripts
with interpreters, but that's the gist of it.
pathexec_run() needs similar attention.
next prev parent reply other threads:[~2020-02-19 8:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-11 17:00 Jacob Vosmaer via supervision
2020-02-19 8:18 ` Jonathan de Boyne Pollard [this message]
[not found] ` <5afb3f20-239b-f09a-c696-fe36d141b2cd@ntlworld.com>
2020-02-19 10:14 ` Laurent Bercot
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=5afb3f20-239b-f09a-c696-fe36d141b2cd@NTLWorld.COM \
--to=j.deboynepollard-newsgroups@ntlworld.com \
--cc=supervision@list.skarnet.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).