Skip to content

Fix duplicated directory descriptor offsets - #1275

Merged
Weiteng Chen (CvvT) merged 2 commits into
mainfrom
fix/issue-1262-shared-directory-position
Sep 4, 2026
Merged

Weiteng Chen (CvvT) merged 2 commits into
mainfrom
fix/issue-1262-shared-directory-position

Conversation

@CvvT

@CvvT Weiteng Chen (CvvT) commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Store directory positions as shared entry metadata so duplicated descriptors share getdents64 and lseek updates. Add a C regression test that runs on both host Linux and LiteBox.

Fix #1262

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

@CvvT
Weiteng Chen (CvvT) marked this pull request as ready for review September 3, 2026 17:36
@wdcui

Copy link
Copy Markdown
Member

Weiteng Chen (@CvvT): a comment from the agent:

Pre-existing race on  Diroff , now also reachable across dup'd fds (file.rs:761-774, 2640-2698)

This isn't a new race introduced by this PR — the underlying pattern already existed:  sys_getdirent64  and directory  lseek  ( SEEK_CUR ) both do a non-atomic read →  read_dir /compute → write of  Diroff  (read the offset under a lock, drop the lock, do the work, then re-acquire a write lock to store the new value). Even before this fix, two threads calling  getdents64 / lseek  concurrently on the same fd could already race and lose an update.

What this PR changes is the blast radius: since  Diroff  is now aliased via  set_entry_metadata  across all fds sharing the same open-file-description entry (which is the whole point of the fix — dup'd fds should share position), that same unprotected read-modify-write window now also lets  getdents64 / lseek  on two different fds (e.g.  dir_fd  and  dup(dir_fd) ) race with each other. Before this change that was impossible, since dup'd fds had fully independent offsets. Concretely: thread A on  dir_fd  and thread B on  dup_fd  can both read  Diroff=0 , both enumerate entries  [0..2) , and whichever writes back last clobbers the other's advance — silently duplicating or skipping entries, which is exactly the shared-position semantics this PR is trying to guarantee. Linux avoids this by holding  f_pos_lock  across the whole operation.

Suggested fix: hold a single exclusive guard on the shared entry across the read →  read_dir /compute → write sequence (or use a CAS loop on  Diroff ) in both  sys_getdirent64  and the directory  lseek  branch, instead of taking separate read and write locks with a gap in between.

@CvvT

Copy link
Copy Markdown
Contributor Author

Weiteng Chen (Weiteng Chen (@CvvT)): a comment from the agent:

Pre-existing race on  Diroff , now also reachable across dup'd fds (file.rs:761-774, 2640-2698)

This isn't a new race introduced by this PR — the underlying pattern already existed:  sys_getdirent64  and directory  lseek  ( SEEK_CUR ) both do a non-atomic read →  read_dir /compute → write of  Diroff  (read the offset under a lock, drop the lock, do the work, then re-acquire a write lock to store the new value). Even before this fix, two threads calling  getdents64 / lseek  concurrently on the same fd could already race and lose an update.

What this PR changes is the blast radius: since  Diroff  is now aliased via  set_entry_metadata  across all fds sharing the same open-file-description entry (which is the whole point of the fix — dup'd fds should share position), that same unprotected read-modify-write window now also lets  getdents64 / lseek  on two different fds (e.g.  dir_fd  and  dup(dir_fd) ) race with each other. Before this change that was impossible, since dup'd fds had fully independent offsets. Concretely: thread A on  dir_fd  and thread B on  dup_fd  can both read  Diroff=0 , both enumerate entries  [0..2) , and whichever writes back last clobbers the other's advance — silently duplicating or skipping entries, which is exactly the shared-position semantics this PR is trying to guarantee. Linux avoids this by holding  f_pos_lock  across the whole operation.

Suggested fix: hold a single exclusive guard on the shared entry across the read →  read_dir /compute → write sequence (or use a CAS loop on  Diroff ) in both  sys_getdirent64  and the directory  lseek  branch, instead of taking separate read and write locks with a gap in between.

I don't think any program would depend on this behavior, but I will try to fix it in a separate PR. Thanks!

@CvvT
Weiteng Chen (CvvT) added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit e286947 Sep 4, 2026
16 of 17 checks passed
@CvvT
Weiteng Chen (CvvT) deleted the fix/issue-1262-shared-directory-position branch September 4, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicated directory descriptors do not share their directory position

2 participants