Skip to content

[pulse] a directory stream created by fdopendir owns its descriptor - #2298

Merged
dulmarod merged 1 commit into
meta:mainfrom
VladimirMakaev:pulse-fdopendir
Nov 4, 2026
Merged

dulmarod merged 1 commit into
meta:mainfrom
VladimirMakaev:pulse-fdopendir

Conversation

@VladimirMakaev

Copy link
Copy Markdown
Contributor

fdopendir(3) takes over the descriptor it is given: the caller must not use or close it, and
closedir releases it. Pulse had no model for fdopendir, so the descriptor kept its obligation
and closing the stream never discharged it.

int list_dir(void) {
  int fd = open("mydir", O_RDONLY);
  if (fd < 0) return -1;
  DIR* dir = fdopendir(fd);
  if (!dir) { close(fd); return -1; }
  closedir(dir); // false positive: resource leak on fd
  return 0;
}

Model the ownership transfer: the stream returned by fdopendir owns the descriptor, and a
failure leaves the descriptor with the caller. Leaks of the stream itself, and of a descriptor
left over by a failed fdopendir, are still reported.

Fixes #1419.

Test plan

New fdopendirClosed_ok and three leak controls in c/pulse/fdopendir.c: the stream dropped
without closedir, the descriptor dropped after a failed fdopendir, and a descriptor never
handed over at all. The codetoanalyze tests pass.

`fdopendir(3)` takes over the descriptor it is given: the caller must not use or close it, and
`closedir` releases it. Pulse had no model for `fdopendir`, so the descriptor kept its obligation
and closing the stream never discharged it.

```c
int list_dir(void) {
  int fd = open("mydir", O_RDONLY);
  if (fd < 0) return -1;
  DIR* dir = fdopendir(fd);
  if (!dir) { close(fd); return -1; }
  closedir(dir); // false positive: resource leak on fd
  return 0;
}
```

Model the ownership transfer: the stream returned by `fdopendir` owns the descriptor, and a
failure leaves the descriptor with the caller. Leaks of the stream itself, and of a descriptor
left over by a failed `fdopendir`, are still reported.

Fixes meta#1419.

## Test plan

New `fdopendirClosed_ok` and three leak controls in `c/pulse/fdopendir.c`: the stream dropped
without `closedir`, the descriptor dropped after a failed `fdopendir`, and a descriptor never
handed over at all. The codetoanalyze tests pass.
@meta-cla meta-cla Bot added the CLA Signed label Nov 4, 2026
@dulmarod
dulmarod merged commit a07b05c into meta:main Nov 4, 2026
7 checks passed
VladimirMakaev added a commit to VladimirMakaev/infer that referenced this pull request Nov 4, 2026
Pulse missed leaks of descriptors from `dup`, `pipe`, `popen` and others, and a failed `fdopen`
closed the descriptor.

`fdopendir` here is upstream's model, not this commit's: facebook/infer meta#2298 (a07b05c)
landed it while this PR was open. The binding and matcher are upstream's, written with this
file's helpers because `FileDescriptor` and `FClose` carry a `Procname.t` on frontier, so
upstream's body does not type-check here.

Expected test output regenerated for the combination with earlier frontier commits: infer/tests/codetoanalyze/c/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp-11

Adapted to earlier frontier commits (integration fixes or overlap with them): infer/src/pulse/PulseModelsC.ml, infer/src/pulse/PulseOperations.ml, infer/tests/codetoanalyze/cpp/pulse/leaks.cpp

Upstream-PR: facebook/infer 2148
PR-Head: e554ac2
PR-Base: 9cc2641
VladimirMakaev added a commit to VladimirMakaev/infer that referenced this pull request Nov 5, 2026
Pulse missed leaks of descriptors from `dup`, `pipe`, `popen` and others, and a failed `fdopen`
closed the descriptor.

`fdopendir` here is upstream's model, not this commit's: facebook/infer meta#2298 (a07b05c)
landed it while this PR was open. The binding and matcher are upstream's, written with this
file's helpers because `FileDescriptor` and `FClose` carry a `Procname.t` on frontier, so
upstream's body does not type-check here.

Expected test output regenerated for the combination with earlier frontier commits: infer/tests/codetoanalyze/c/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp-11

Adapted to earlier frontier commits (integration fixes or overlap with them): infer/src/pulse/PulseModelsC.ml, infer/src/pulse/PulseOperations.ml, infer/tests/codetoanalyze/cpp/pulse/leaks.cpp

Upstream-PR: facebook/infer 2148
PR-Head: e554ac2
PR-Base: 9cc2641
VladimirMakaev added a commit to VladimirMakaev/infer that referenced this pull request Nov 5, 2026
Pulse missed leaks of descriptors from `dup`, `pipe`, `popen` and others, and a failed `fdopen`
closed the descriptor.

`fdopendir` here is upstream's model, not this commit's: facebook/infer meta#2298 (a07b05c)
landed it while this PR was open. The binding and matcher are upstream's, written with this
file's helpers because `FileDescriptor` and `FClose` carry a `Procname.t` on frontier, so
upstream's body does not type-check here.

Expected test output regenerated for the combination with earlier frontier commits: infer/tests/codetoanalyze/c/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp-11

Adapted to earlier frontier commits (integration fixes or overlap with them): infer/src/pulse/PulseModelsC.ml, infer/src/pulse/PulseOperations.ml, infer/tests/codetoanalyze/cpp/pulse/leaks.cpp

Upstream-PR: facebook/infer 2148
PR-Head: e554ac2
PR-Base: 9cc2641
VladimirMakaev added a commit to VladimirMakaev/infer that referenced this pull request Nov 6, 2026
Pulse missed leaks of descriptors from `dup`, `pipe`, `popen` and others, and a failed `fdopen`
closed the descriptor.

`fdopendir` here is upstream's model, not this commit's: facebook/infer meta#2298 (a07b05c)
landed it while this PR was open. The binding and matcher are upstream's, written with this
file's helpers because `FileDescriptor` and `FClose` carry a `Procname.t` on frontier, so
upstream's body does not type-check here.

Expected test output regenerated for the combination with earlier frontier commits: infer/tests/codetoanalyze/c/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp, infer/tests/codetoanalyze/cpp/pulse/issues.exp-11

Adapted to earlier frontier commits (integration fixes or overlap with them): infer/src/pulse/PulseModelsC.ml, infer/src/pulse/PulseOperations.ml, infer/tests/codetoanalyze/cpp/pulse/leaks.cpp

Upstream-PR: facebook/infer 2148
PR-Head: e554ac2
PR-Base: 9cc2641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False Positive Resource Leak with fdopendir(3) [C]

2 participants