Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix DirReaderPosix close same fd twice #2415

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Conversation

ehds
Copy link
Contributor

@ehds ehds commented Oct 17, 2023

What problem does this PR solve?

Issue Number:

Problem Summary:
DirReaderPosix 析构时关闭只应调用 closedir(dir_) , 不应同时调用 close(fd_), 否则会导致同一个 fd 被close 两次。

Upon calling closedir() the file descriptor is closed.

https://man.freebsd.org/cgi/man.cgi?query=fdopendir&sektion=3
https://pubs.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html

PoC :
如果在 close(fd_)closedir(dir_) 之间有新的 open 操作,fd_ 可能会被复用,则 closedir(dir_) 导致该新 open 的 fd_ 失效,后续对 fd_ 的操作将会导致 errno = 9 (Bad File Descriptor) 错误。

ps: 实际情况可能是在多个线程中触发,这里以单线程代码说明:

#include <stdio.h>
#include <dirent.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdlib.h>
#include <errno.h>
#include <unistd.h>
#include <assert.h>
#include <string.h>


int main(int argc, char *argv[])
{
    DIR *d;
    struct dirent *dp;
    int dfd = 0;

    if ((d = fdopendir((dfd = open("./tmp", O_RDONLY)))) == NULL) {
        fprintf(stderr, "Cannot open ./tmp directory\n");
       exit(1);
    }

    // close dfd first,  dfd = 3.
    close(dfd); 

    // then open a new file, ffd maybe be reused, ffd = 3.
    int ffd = open("./tmp/testfile", O_RDONLY); 
    assert(ffd > 0);

    closedir(d); // note this implicitly closes dfd = 3, so ffd is invalid. 
    
    // read ffd would be failed.
    int rc = read(ffd, 0, 2);
    if(rc < 0) {
        printf("Read failed, error %d (%s)\n", errno, strerror(errno)); // errno = 9.
    }
    return 0;
}

What is changed and the side effects?

Changed:
~DirReaderUnix() 中只需调用 closedir(dir_) 即可.

Side effects:

  • Performance effects(性能影响):
    No
  • Breaking backward compatibility(向后兼容性):
    No

Check List:

  • Please make sure your changes are compilable(请确保你的更改可以通过编译).
  • When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
  • Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

@ehds ehds force-pushed the fix-unix-dir-close branch 2 times, most recently from 0207571 to e2fc613 Compare October 17, 2023 17:34
@ehds ehds force-pushed the fix-unix-dir-close branch from e2fc613 to ddfd893 Compare October 17, 2023 17:34
@wasphin
Copy link
Member

wasphin commented Oct 17, 2023

LGTM

1 similar comment
@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 18, 2023

LGTM

@ehds ehds force-pushed the fix-unix-dir-close branch from 965a9e7 to 1245d5c Compare October 18, 2023 03:50
@lorinlee lorinlee merged commit ba5271a into apache:master Oct 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants