* lib/openat2.c (do_openat2): Don’t assume that openat (..., "symlink", O_DIRECTORY | O_NOFOLLOW | ...) will fail with ELOOP. The O_DIRECTORY check might be done first, so it might fail with ENOTDIR. Problem discovered on Solaris 10. * tests/test-openat2.c (do_prepare_symlinks, do_test_resolve) (do_test_basic): Test for the portability bug. --- ChangeLog | 8 +++++++ lib/openat2.c | 55 ++++++++++++++++++++++++++------------------ tests/test-openat2.c | 28 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 23 deletions(-)
diff --git a/ChangeLog b/ChangeLog index d0d5ccba02..925e9597ed 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2025-11-14 Paul Eggert <[email protected]> + openat2: port to O_DIRECTORY before O_NOFOLLOW + * lib/openat2.c (do_openat2): Don’t assume that openat (..., + "symlink", O_DIRECTORY | O_NOFOLLOW | ...) will fail with ELOOP. + The O_DIRECTORY check might be done first, so it might fail with + ENOTDIR. Problem discovered on Solaris 10. + * tests/test-openat2.c (do_prepare_symlinks, do_test_resolve) + (do_test_basic): Test for the portability bug. + openat2-tests: port to O_RDONLY != 0 * tests/test-openat2.c (do_test_resolve): Don’t assume O_RDONLY == 0. diff --git a/lib/openat2.c b/lib/openat2.c index 4ad4e88f6c..ba69cf6704 100644 --- a/lib/openat2.c +++ b/lib/openat2.c @@ -331,8 +331,38 @@ do_openat2 (int *fd, char const *filename, if (subfd < 0) { - if (maxlinks <= 0 || errno != ELOOP) - return negative_errno (); + int openerr = negative_errno (); + if (! ((openerr == -ELOOP) + | (!!(subflags & O_DIRECTORY) & (openerr == -ENOTDIR)))) + return openerr; + + /* Likely a symlink. + Try to read symlink contents into buffer start. + But if the root prefix might be needed, + leave room for it at buffer start. */ + idx_t rootlen = f - g; + char *slink; + ssize_t slinklen; + for (idx_t more = rootlen + 1; ; more = bufsize - f + 1) + { + bufsize = maybe_grow (buf, bufsize, stackbuf, more, f); + if (bufsize < 0) + return bufsize; + e = *buf + bufsize; + slink = *buf + rootlen; + idx_t slinksize = bufsize - f - rootlen; + slinklen = readlinkat (*fd, &e[-f], slink, slinksize); + if (slinklen < 0) + { + int readlinkerr = negative_errno (); + return readlinkerr == -EINVAL ? -ENOTDIR : readlinkerr; + } + if (slinklen < slinksize) + break; + } + + if (maxlinks <= 0) + return -ELOOP; maxlinks--; /* A symlink and the symlink loop count is not exhausted. @@ -359,27 +389,6 @@ do_openat2 (int *fd, char const *filename, } #endif - /* Read symlink contents into buffer start. - But if the root prefix might be needed, - leave room for it at buffer start. */ - idx_t rootlen = f - g; - char *slink; - ssize_t slinklen; - for (idx_t more = rootlen + 1; ; more = bufsize - f + 1) - { - bufsize = maybe_grow (buf, bufsize, stackbuf, more, f); - if (bufsize < 0) - return bufsize; - e = *buf + bufsize; - slink = *buf + rootlen; - idx_t slinksize = bufsize - f - rootlen; - slinklen = readlinkat (*fd, &e[-f], slink, slinksize); - if (slinklen < 0) - return negative_errno (); - if (slinklen < slinksize) - break; - } - /* Compute KEPT, the number of trailing bytes in the file name that will be appended to the symlink contents. */ idx_t kept; diff --git a/tests/test-openat2.c b/tests/test-openat2.c index 67b132674c..08cbac0ea5 100644 --- a/tests/test-openat2.c +++ b/tests/test-openat2.c @@ -89,6 +89,8 @@ do_prepare_symlinks () Construct a test directory with the following structure: temp_dir/ + |- dirlink -> subdir + |- dirlinkslash -> subdir/ |- escaping_link -> / |- escaping_link_2 -> escaping_link |- some_file @@ -98,6 +100,8 @@ do_prepare_symlinks () |- some_file */ + ASSERT (symlinkat ("subdir", dfd, "dirlink") == 0); + ASSERT (symlinkat ("subdir/", dfd, "dirlinkslash") == 0); ASSERT (symlinkat ("/", dfd, "escaping_link") == 0); ASSERT (symlinkat ("escaping_link", dfd, "escaping_link_2") == 0); ASSERT (symlinkat ("some_file/invalid", dfd, "invalid_link") == 0); @@ -345,6 +349,28 @@ do_test_resolve (void) { int fd; + fd = openat2 (dfd, + "dirlink", + (&(struct open_how) + { + .flags = O_RDONLY | O_DIRECTORY, + .resolve = RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS, + }), + sizeof (struct open_how)); + ASSERT (is_nofollow_error (errno)); + ASSERT (fd == -1); + + fd = openat2 (dfd, + "dirlinkslash", + (&(struct open_how) + { + .flags = O_RDONLY | O_DIRECTORY, + .resolve = RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS, + }), + sizeof (struct open_how)); + ASSERT (is_nofollow_error (errno)); + ASSERT (fd == -1); + /* ESCAPING_LINK links to /tmp, which escapes the temporary test directory. */ fd = openat2 (dfd, @@ -485,6 +511,8 @@ do_test_basic () ASSERT (unlinkat (dfd, "some-file", 0) == 0); + ASSERT (unlinkat (dfd, "dirlink", 0) == 0); + ASSERT (unlinkat (dfd, "dirlinkslash", 0) == 0); ASSERT (unlinkat (dfd, "escaping_link", 0) == 0); ASSERT (unlinkat (dfd, "escaping_link_2", 0) == 0); ASSERT (unlinkat (dfd, "invalid_link", 0) == 0); -- 2.51.0
