Bug 276632 - LLVM std::filesystem::remove_all fails to handle dangling symlinks
Summary: LLVM std::filesystem::remove_all fails to handle dangling symlinks
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Dimitry Andric
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-26 02:22 UTC by Mark Johnston
Modified: 2024-02-08 20:12 UTC (History)
2 users (show)

See Also:
linimon: mfc-stable14?
linimon: mfc-stable13?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Johnston freebsd_committer freebsd_triage 2024-01-26 02:22:11 UTC
remove_all_impl() will attempt to remove a file specified by its path, recursing if it encounters a directory.

It opens the file with O_CLOEXEC | O_RDONLY | O_DIRECTORY | O_NOFOLLOW, which for a dangling symlink returns EMLINK on FreeBSD, contradicting POSIX (which says it should return ELOOP in this scenario).  Bug 214633 notes this problem, but fixing it would break compatibility.

remove_all_impl() however only handles ENOTDIR and ELOOP, so it incorrectly returns an error in this situation on FreeBSD.  We should extend it to check for EMLINK as well.
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2024-01-26 02:35:22 UTC
https://github.com/llvm/llvm-project/pull/79540
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2024-01-29 16:36:15 UTC
This is fixed upstream now.  Dmitry, would you be willing to import the patch?
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-01-29 17:29:04 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ee14a9725d73150e89367550206803fe36ae3089

commit ee14a9725d73150e89367550206803fe36ae3089
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2024-01-29 17:26:48 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2024-01-29 17:26:48 +0000

    Merge commit 4a39d0890894 from llvm-project (by Mark Johnston):

      [libc++] Fix filesystem::remove_all() on FreeBSD (#79540)

      remove_all_impl() opens the target path with O_NOFOLLOW, which fails if
      the target is a symbolic link. On FreeBSD, rather than returning ELOOP,
      openat() returns EMLINK. This is unlikely to change for compatibility
      reasons, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214633 .

      Thus, check for EMLINK as well.

    Reported by:    markj
    PR:             276632
    MFC after:      3 days

 contrib/llvm-project/libcxx/src/filesystem/operations.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2024-02-08 02:52:43 UTC
^Triage: assign to committer.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-02-08 19:26:43 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=7d91a95f825ae79245be5c16acecae254c51d142

commit 7d91a95f825ae79245be5c16acecae254c51d142
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2024-01-29 17:26:48 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2024-02-08 19:25:52 +0000

    Merge commit 4a39d0890894 from llvm-project (by Mark Johnston):

      [libc++] Fix filesystem::remove_all() on FreeBSD (#79540)

      remove_all_impl() opens the target path with O_NOFOLLOW, which fails if
      the target is a symbolic link. On FreeBSD, rather than returning ELOOP,
      openat() returns EMLINK. This is unlikely to change for compatibility
      reasons, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214633 .

      Thus, check for EMLINK as well.

    Reported by:    markj
    PR:             276632
    MFC after:      3 days

    (cherry picked from commit ee14a9725d73150e89367550206803fe36ae3089)

 contrib/llvm-project/libcxx/src/filesystem/operations.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2024-02-08 19:28:44 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d5d6b3938d842529a01bdc6404c77e84213b0192

commit d5d6b3938d842529a01bdc6404c77e84213b0192
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2024-01-29 17:26:48 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2024-02-08 19:27:38 +0000

    Merge commit 4a39d0890894 from llvm-project (by Mark Johnston):

      [libc++] Fix filesystem::remove_all() on FreeBSD (#79540)

      remove_all_impl() opens the target path with O_NOFOLLOW, which fails if
      the target is a symbolic link. On FreeBSD, rather than returning ELOOP,
      openat() returns EMLINK. This is unlikely to change for compatibility
      reasons, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214633 .

      Thus, check for EMLINK as well.

    Reported by:    markj
    PR:             276632
    MFC after:      3 days

    (cherry picked from commit ee14a9725d73150e89367550206803fe36ae3089)

 contrib/llvm-project/libcxx/src/filesystem/operations.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)