microsoft / microsoft/litebox

Serialize operations on directory position

Open
#1,290 0 comments 0 reactions 1 assignee Claimed by @CvvT View on GitHub
Dominant language
Rust
Stars
2.7k
Forks
144
Avg merge
12h 21m
Merged PRs (30d)
146

Description

@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.

_Originally posted by @wdcui in https://github.com/microsoft/litebox/issues/1275#issuecomment-5533403537_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.