I almost got bitten by this. access, eaccess, faccessat will ALWAYS dereference symbolic links. (though Linux allows the flag AT_SYMLINK_NOFOLLOW to be applied to faccessat) Could a note like this be added to the man page? "access, eaccess, faccessat will ALWAYS dereference symbolic links. If you need to refer to the symbolic link itself, you'll have to instead use stat(2)"
*tumbleweed*
Thanks Gordon. Would it have been better for me to submit a review instead, or is that just for committers? Cheers, Jamie
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=421025a274fb5759b3ecc8bdb30b24db830b45ae commit 421025a274fb5759b3ecc8bdb30b24db830b45ae Author: Gordon Bergling <gbe@FreeBSD.org> AuthorDate: 2024-05-13 08:40:12 +0000 Commit: Gordon Bergling <gbe@FreeBSD.org> CommitDate: 2024-05-13 08:40:12 +0000 access.2: Mention that lstat(2) should be used for symbolic links access(), eaccess() and faccessat() will always dereference symbolic links. So add a note in the manual page, that lstat(2) should be used in the case of symbolic links. PR: 262895 Reviewed by: gbe, pauamma_gundo.com MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D44890 lib/libsys/access.2 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Jamie, how exactly did you “almost get bitten” by this? `access()` etc always dereference the symbolic link because the question they answer is “what would happen if I tried to `open()` this path” and `open()` always dereferences symbolic links. Furthermore, as is already noted in the manual page, you should probably not be using `access()` at all for reasons entirely unrelated to whether or not it follows symbolic links.
(In reply to Dag-Erling Smørgrav from comment #4) Hi. Thanks for the feedback. I could be wrong, but I'll explain my situation as far as I can remember: I wasn't trying to open the file - I wanted to check the existence of any entry with that name in a directory before renaming a file to said name. Rather than "do it and see", if an entry of the destination name already existed, I wanted the options presented to the user prior to the rename to be changed appropriately. I saw at the end of the manpage for access(2) : "access() remains useful for providing clues to users as to whether operations make sense for particular filesystem objects (e.g. 'delete' menu item only highlighted in a writable folder ... avoiding interpretation of the st_mode bits that the application might not understand -- e.g. in the case of AFS). It also allows a cheaper file existence test than stat(2)." Going by that, access seemed the best option for the job, but of course, it failed when I had a softlink in the directory that pointed to a non-existent file, causing an unfriendly error rather than the result I wanted. OK, hardly the end of the world, but I had expected a test for whether an entry exists within a directory to not fail in this case, hence why I suggested adding the bit about softlinks to that paragraph (Incidentally, I meant "lstat" not "stat" - I have already corrected it in the review)
OK, I think the proper fix here would be to remove the sentence about `access()` being a cheaper existence test, because it's not an existence test at all.
(In reply to Dag-Erling Smørgrav from comment #6) Fair enough. Obviously, even in my use case, there could be a potential race condition, so I suppose its use as an existence test can only be considered to give historical information. Might it be an idea when you remove that line to suggest in its place what would be the best alternative to use? I think some people will be more likely to follow the security advice if they are given an alternative!
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a4be1eb21165d7aedae9dc6634528619ff10d025 commit a4be1eb21165d7aedae9dc6634528619ff10d025 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-05-21 22:35:22 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-21 22:35:22 +0000 access(2): Discourage use of these system calls. Fixes: 421025a274fb PR: 262895 MFC after: 3 days Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D45240 lib/libsys/access.2 | 105 +++++++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 55 deletions(-)
(In reply to Dag-Erling Smørgrav from comment #8) Thank-you. That's much clearer.
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=79b39f23e1341bdc3ab9c6a08e1506d40b8b6dbc commit 79b39f23e1341bdc3ab9c6a08e1506d40b8b6dbc Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-05-21 22:35:22 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:41:32 +0000 access(2): Discourage use of these system calls. Fixes: 421025a274fb PR: 262895 MFC after: 3 days Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D45240 (cherry picked from commit a4be1eb21165d7aedae9dc6634528619ff10d025) lib/libc/sys/access.2 | 105 ++++++++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 55 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f075b6113726d74342d4af15dfab60709f50b859 commit f075b6113726d74342d4af15dfab60709f50b859 Author: Gordon Bergling <gbe@FreeBSD.org> AuthorDate: 2024-05-13 08:40:12 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:41:32 +0000 access.2: Mention that lstat(2) should be used for symbolic links access(), eaccess() and faccessat() will always dereference symbolic links. So add a note in the manual page, that lstat(2) should be used in the case of symbolic links. PR: 262895 Reviewed by: gbe, pauamma_gundo.com MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D44890 (cherry picked from commit 421025a274fb5759b3ecc8bdb30b24db830b45ae) lib/libc/sys/access.2 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=55cd52a4418d79962ebd81f34544a3747db9e031 commit 55cd52a4418d79962ebd81f34544a3747db9e031 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-05-21 22:35:22 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:43:49 +0000 access(2): Discourage use of these system calls. Fixes: 421025a274fb PR: 262895 MFC after: 3 days Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D45240 (cherry picked from commit a4be1eb21165d7aedae9dc6634528619ff10d025) lib/libc/sys/access.2 | 105 ++++++++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 55 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=78ea1fcf29a780fb6bd90e41772d5a3751fb56d7 commit 78ea1fcf29a780fb6bd90e41772d5a3751fb56d7 Author: Gordon Bergling <gbe@FreeBSD.org> AuthorDate: 2024-05-13 08:40:12 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:43:49 +0000 access.2: Mention that lstat(2) should be used for symbolic links access(), eaccess() and faccessat() will always dereference symbolic links. So add a note in the manual page, that lstat(2) should be used in the case of symbolic links. PR: 262895 Reviewed by: gbe, pauamma_gundo.com MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D44890 (cherry picked from commit 421025a274fb5759b3ecc8bdb30b24db830b45ae) lib/libc/sys/access.2 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
A commit in branch releng/14.1 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8766ae10d3b792caf7c31b583afcaf43b1a6543f commit 8766ae10d3b792caf7c31b583afcaf43b1a6543f Author: Gordon Bergling <gbe@FreeBSD.org> AuthorDate: 2024-05-13 08:40:12 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:45:49 +0000 access.2: Mention that lstat(2) should be used for symbolic links access(), eaccess() and faccessat() will always dereference symbolic links. So add a note in the manual page, that lstat(2) should be used in the case of symbolic links. PR: 262895 Reviewed by: gbe, pauamma_gundo.com MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D44890 (cherry picked from commit 421025a274fb5759b3ecc8bdb30b24db830b45ae) (cherry picked from commit f075b6113726d74342d4af15dfab60709f50b859) Approved by: re (cperciva) lib/libc/sys/access.2 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
A commit in branch releng/14.1 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fb18c369c32d746569e36ae7dca7dad43bcc118e commit fb18c369c32d746569e36ae7dca7dad43bcc118e Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-05-21 22:35:22 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-23 02:45:55 +0000 access(2): Discourage use of these system calls. Fixes: 421025a274fb PR: 262895 MFC after: 3 days Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D45240 (cherry picked from commit a4be1eb21165d7aedae9dc6634528619ff10d025) (cherry picked from commit 79b39f23e1341bdc3ab9c6a08e1506d40b8b6dbc) Approved by: re (cperciva) lib/libc/sys/access.2 | 105 ++++++++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 55 deletions(-)