Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Double free on deinitialization after mounting using PHYSFS_mountHandle and leaving a file handle unclosed. #88

Open
Fothsid opened this issue Aug 23, 2024 · 0 comments
Assignees

Comments

@Fothsid
Copy link

Fothsid commented Aug 23, 2024

When an archive is mounted using PHYSFS_mountHandle, open a file from said archive and don't close it, then PHYSFS will SEGFAULT upon deinitialization. This doesn't happen when an archive is mounted using PHYSFS_mount.

Personally I encountered this issue because I have to mount archives inside of an archive inside of an archive in my personal project. (yes, it's cursed. Nested PS2 .AFS archives, which are just glorified header and a table of offsets to files, no compression). First layer is mounted normally using PHYSFS_mount, second and third layers are mounted using PHYSFS_mountHandle.

Attached below are a sample program to reproduce the issue and a sample archive to use with the program.

After some debugging I found out that it destroys the second layer archive twice. First time it is freed when closeFileHandleList is iterating through the file handles, then it attempts to destroy it the second time when it gets to the third layer archive file handle, destroying the IO of which results in trying to close the parent archive file in handleIo_destroy. My current fix for this issue for my personal project is to remove the PHYSFS_close call in handleIo_destroy and to add a deallocation of the file buffer to closeFileHandleList to avoid memory leaks. I'm not really confident with this being the right solution, so I'm not posting it as a pull request at the moment.

Sample program in C to reproduce the issue
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <physfs.h>

int main(int argc, char* argv[]) {
    if (!PHYSFS_init(argv[0])) {
        printf("0 %s\n", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
        return 1;
    }

    PHYSFS_File* file;

    if (!PHYSFS_mount(".", "mnt", 0)) {
        printf("1 %s\n", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
        return 1;
    }
    
    file = PHYSFS_openRead("mnt/test.zip");

    if (!file) {
        printf("2 %s\n", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
        return 1;
    }

    if (!PHYSFS_mountHandle(file, "mnt/test.zip", "zip", 0)) {
        printf("3 %s\n", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
        return 1;
    }

    file = PHYSFS_openRead("zip/hello.txt");
    if (!file) {
        printf("4 %s\n", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
        return 1;
    }

    int fileLen = PHYSFS_fileLength(file);
    printf("file length: %d\n", fileLen);

    void* data = malloc(fileLen+1);
    if (!data) {
        printf("5 malloc failed (%d)", fileLen+1);
        return 1;
    }
    memset(data, 0, fileLen+1);
    PHYSFS_readBytes(file, data, fileLen);
    
    printf("%s\n", data);
    
    // Do not close the hello.txt file on purpose.
    PHYSFS_deinit(); // <-- this will segfault.
    return 0;
}

test.zip

@icculus icculus self-assigned this Aug 24, 2024
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

No branches or pull requests

2 participants