On 09/16/2015 04:17 PM, Jonathan Wakely wrote:
On 16/09/15 16:04 -0600, Martin Sebor wrote:
Tested powerpc64le-linux, x86_64-dragonfly4.1 and x86_64-netbsd5.1,
do you see any reason not to commit this for now?
I see only a couple of potential problems: a missing test for
PATH_MAX in the unlikely event it's not defined (or is obscenely
In the current patch _GLIBCXX_USE_REALPATH won't be defined unless:
#elif _XOPEN_VERSION >= 700 || defined(PATH_MAX)
so if it's defined and _XOPEN_VERSION < 700 then we know PATH_MAX must
be defined (otherwise _GLIBCXX_USE_REALPATH wouldn't be).
large), and a missing check to avoid infinite loops due to symlinks.
I thought about keeping track of where I'd been while expanding
symlinks, but then realised this will do it:
if (!exists(pa, ec))
{
fail(ENOENT);
return result;
}
// else we can assume no unresolvable symlink loops
If there's a symlink loop then exists(pa) will fail with ELOOP, and we
won't try to resolve it by hand.
And then after each step in the while(!cmpts.empty()) loop I also have
a check for !exists(result, ec), which should even handle the case
where the filesystem changes after the initial exists() call so that a
loop is introduced while we're canonicalising the path.
I obviously didn't read the patch carefully enough and missed
both the PATH_MAX check and the loop comment.
I see now the first exists test will detect symlink loops in
the original path. But I'm not convinced there isn't a corner
case that's subject to a TOCTOU race condition between the first
exists test and the while loop during which a symlink loop can
be introduced.
Suppose we call the function with /foo/bar as an argument and
the path exists and contains no symlinks. result is / and cmpts
is set to { foo, bar }. Just as the loop is entered, /foo/bar
is replaced with a symlink containing /foo/bar. The loop then
proceeds like so:
1. The first iteration removes foo from cmpts and sets result
to /foo. cmpts is { bar }.
2. The second iteration removes bar from cmpts, sets result to
/foo/bar, determines it's a symlink, reads its contents, sees
it's an absolute pathname and replaces result with /. It then
inserts the symlink's components { foo, bar } into cmpts. cmpts
becomes { foo, bar }. exists(result) succeeds.
3. The next iteration of the loop has the same initial state
as the first.
But I could have very easily missed something that takes care
of this corner case. If I did, sorry for the false alarm!
Martin
