Skip to content

fix(libcglue): fix descriptor table handling in dup, dup2 and open - #956

Closed
PS2HomeDeveloper wants to merge 5 commits into
ps2dev:masterfrom
PS2HomeDeveloper:patch-10
Closed

PS2HomeDeveloper wants to merge 5 commits into
ps2dev:masterfrom
PS2HomeDeveloper:patch-10

Conversation

@PS2HomeDeveloper

Copy link
Copy Markdown
Contributor

Description

This fixes three bugs in the file descriptor table of libcglue
(ee/libcglue/src/fdman.c and dup2() in glue.c).

1. dup() and dup2() of a descriptor that is itself a duplicate

__fdman_get_dup_descriptor() and __fdman_get_dup2_descriptor()
stored &__descriptor_data_pool[fd], the pool entry whose index equals
the descriptor number, instead of the entry the descriptor actually uses
(__descriptormap[fd]). Those are the same only when fd was not
created by a previous dup(). For dup(dup(fd)) the new descriptor
pointed to an unused, empty pool entry, so any call on it failed (a read
returned -1 with ENOSYS). They now copy __descriptormap[fd].

2. open() could reuse a pool entry still used by a duplicate

__fdman_get_new_descriptor() always paired the free descriptor number
i with pool entry i. If the original descriptor was closed while a
duplicate was still open, that pool entry was still in use, and the next
open() filled it with the new file, so the duplicate suddenly referred
to the other file (and the first file's handle was lost). It now picks a
pool entry that no descriptor points to, clears it, and starts its
reference count at 1. The search is at most 64 x 64 comparisons and only
runs when a descriptor is opened.

3. Missing upper bound check in dup2()

dup2() only rejected newfd < 0. With newfd >= __FILENO_MAX (64) it
read __descriptormap[newfd] and then wrote the new entry there, outside
of the table. It now fails with EBADF, and the same check was added in
__fdman_get_dup2_descriptor().

Impact

  • No behavior change for descriptors that are not duplicated.
  • dup()/dup2() on a duplicate now work, and closing the original
    descriptor no longer breaks its duplicates when new files are opened.
  • dup2() with an out-of-range descriptor no longer corrupts memory.

Testing

Tested on a real PS2 console, booting a test ELF from a USB drive. The
test creates two files (one filled with A, one with B) and checks:

  • T1: read() on dup(dup(fd)) returns the file's data.
  • T2: after dup(fd) and close(fd), opening the second file must not
    change what the duplicate reads.
  • T3: dup2(fd, 64) must fail with EBADF.

The same program was built twice, once without and once with this change.
The dup() and dup2() code was the same in both builds apart from
this change (the first build used libcglue from master plus two of my
own earlier patches, #950 and #954, which do not touch this code).

  • Before: T1 failed (read returned -1, errno 88), T2 failed (the
    duplicate read the data of the other file), T3 failed (dup2 returned
    64 instead of failing).
  • After: all three pass.
Result before the change
dup/dup2 test log
log path: mass:APPS/dup_result.txt
argc=1

start (waited 1 s)
T1 dup of a dup: fd=3 dup=4 dup2=5, read returned -1 errno=88 data='' FAIL
T2 original closed, other file opened (fd 3): dup (fd 4) reads 'BBBB' (expected AAAA), new fd reads 'BBBB' (expected BBBB) FAIL
T3 about to call dup2(fd, 64); if no result follows, the console crashed or hung here
T3 dup2(fd, 64) returned 64, errno=9 FAIL (expected -1 and errno 9)
summary: T1=FAIL T2=FAIL T3=FAIL
Result after the change
dup/dup2 test log
log path: mass:APPS/dup_result.txt
argc=1

start (waited 1 s)
T1 dup of a dup: fd=3 dup=4 dup2=5, read returned 4 errno=0 data='AAAA' PASS
T2 original closed, other file opened (fd 3): dup (fd 4) reads 'AAAA' (expected AAAA), new fd reads 'BBBB' (expected BBBB) PASS
T3 about to call dup2(fd, 64); if no result follows, the console crashed or hung here
T3 dup2(fd, 64) returned -1, errno=9 PASS (expected -1 and errno 9)
summary: T1=PASS T2=PASS T3=PASS
Test program (main.c)
#define NEWLIB_PORT_AWARE
#include <stdio.h>
#include <stdarg.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
#include <dirent.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <kernel.h>
#include <sifrpc.h>
#include <loadfile.h>
#include <iopcontrol.h>
#include <sbv_patches.h>
#include <fileXio_rpc.h>
#include <debug.h>

#define IRX_EXTERN(name) \
    extern unsigned char name##_irx[]; \
    extern unsigned int size_##name##_irx

IRX_EXTERN(iomanX);
IRX_EXTERN(fileXio);
IRX_EXTERN(usbd);
IRX_EXTERN(bdm);
IRX_EXTERN(bdmfs_fatfs);
IRX_EXTERN(usbmass_bd);

static char g_log_path[256] = "mass:/dup_result.txt";

/* The log is placed next to the ELF, as in the argv[0] directory. */
static void log_init(int argc, char *argv[])
{
    FILE *f;
    char *slash;

    if (argc > 0 && argv[0] != NULL && strncmp(argv[0], "mass", 4) == 0) {
        strncpy(g_log_path, argv[0], sizeof(g_log_path) - 1);
        g_log_path[sizeof(g_log_path) - 1] = '\0';
        slash = strrchr(g_log_path, '/');
        if (slash != NULL && (slash - g_log_path) < (int)sizeof(g_log_path) - 24)
            strcpy(slash + 1, "dup_result.txt");
        else
            strcpy(g_log_path, "mass:/dup_result.txt");
    }

    f = fopen(g_log_path, "w"); /* truncate the file on every new run */
    if (f != NULL) {
        fprintf(f, "dup/dup2 test log\nlog path: %s\nargc=%d\n\n", g_log_path, argc);
        fclose(f);
    } else {
        scr_printf("cannot create log: %s\n", g_log_path);
    }
}

/* Print on screen first, then append one line to the log and close it at once. */
static void log_printf(const char *fmt, ...)
{
    char line[256];
    FILE *f;
    va_list args;

    va_start(args, fmt);
    vsnprintf(line, sizeof(line), fmt, args);
    va_end(args);

    scr_printf("%s", line);
    f = fopen(g_log_path, "a");
    if (f != NULL) {
        fputs(line, f);
        fclose(f);
    }
}

static void load_irx(const char *name, void *buf, unsigned int size)
{
    int ret = 0;
    int id = SifExecModuleBuffer(buf, size, 0, NULL, &ret);
    scr_printf("%s: id=%d ret=%d\n", name, id, ret);
}

/* Create a 1024 byte file filled with one letter. */
static int make_file(const char *path, char c)
{
    char data[512];
    int fd, i;

    memset(data, c, sizeof(data));
    fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
    if (fd < 0)
        return -1;
    for (i = 0; i < 2; i++)
        write(fd, data, sizeof(data));
    close(fd);
    return 0;
}

int main(int argc, char *argv[])
{
    char pa[] = "mass:/dup_a.tmp";
    char pb[] = "mass:/dup_b.tmp";
    char buf[8], buf2[8];
    int i, fa, fb, d1, d2, da, r, r2, e;
    int t1 = 0, t2 = 0, t3 = 0;

    SifInitRpc(0);
    while (!SifIopReset("", 0)) {}
    while (!SifIopSync()) {}
    SifInitRpc(0);
    sbv_patch_enable_lmb();
    sbv_patch_disable_prefix_check();
    init_scr();

    load_irx("iomanX", iomanX_irx, size_iomanX_irx);
    load_irx("fileXio", fileXio_irx, size_fileXio_irx);
    fileXioInit();
    load_irx("usbd", usbd_irx, size_usbd_irx);
    load_irx("bdm", bdm_irx, size_bdm_irx);
    load_irx("bdmfs_fatfs", bdmfs_fatfs_irx, size_bdmfs_fatfs_irx);
    load_irx("usbmass_bd", usbmass_bd_irx, size_usbmass_bd_irx);

    /* Wait for the USB drive to show up, up to 30 seconds. */
    for (i = 0; i < 30; i++) {
        DIR *d = opendir("mass:/");
        if (d != NULL) {
            closedir(d);
            break;
        }
        scr_printf("waiting for mass: %d\n", i);
        sleep(1);
    }
    if (i == 30) {
        scr_printf("mass: not found, stopping\n");
        SleepThread();
    }

    log_init(argc, argv);
    log_printf("start (waited %d s)\n", i);

    if (make_file(pa, 'A') < 0 || make_file(pb, 'B') < 0) {
        log_printf("cannot create the test files, errno=%d\n", errno);
        SleepThread();
    }

    /* T1: a duplicate of a duplicate must read the same file. */
    fa = open(pa, O_RDONLY);
    d1 = dup(fa);
    d2 = dup(d1);
    memset(buf, 0, sizeof(buf));
    errno = 0;
    r = read(d2, buf, 4);
    e = errno;
    t1 = (fa >= 0 && d1 >= 0 && d2 >= 0 && r == 4 && memcmp(buf, "AAAA", 4) == 0);
    log_printf("T1 dup of a dup: fd=%d dup=%d dup2=%d, read returned %d errno=%d data='%.4s' %s\n",
               fa, d1, d2, r, e, buf, t1 ? "PASS" : "FAIL");
    close(d2);
    close(d1);
    close(fa);

    /* T2: closing the original and opening another file must not change what the duplicate reads. */
    fa = open(pa, O_RDONLY);
    da = dup(fa);
    close(fa);
    fb = open(pb, O_RDONLY);
    memset(buf, 0, sizeof(buf));
    memset(buf2, 0, sizeof(buf2));
    r = read(da, buf, 4);
    r2 = read(fb, buf2, 4);
    t2 = (r == 4 && memcmp(buf, "AAAA", 4) == 0 && r2 == 4 && memcmp(buf2, "BBBB", 4) == 0);
    log_printf("T2 original closed, other file opened (fd %d): dup (fd %d) reads '%.4s' (expected AAAA), new fd reads '%.4s' (expected BBBB) %s\n",
               fb, da, buf, buf2, t2 ? "PASS" : "FAIL");
    close(da);
    close(fb);

    /* T3: dup2() to a descriptor number outside the table (64 entries) must fail with EBADF. */
    fa = open(pa, O_RDONLY);
    log_printf("T3 about to call dup2(fd, 64); if no result follows, the console crashed or hung here\n");
    errno = 0;
    r = dup2(fa, 64);
    e = errno;
    t3 = (r == -1 && e == EBADF);
    log_printf("T3 dup2(fd, 64) returned %d, errno=%d %s (expected -1 and errno %d)\n",
               r, e, t3 ? "PASS" : "FAIL", EBADF);

    log_printf("summary: T1=%s T2=%s T3=%s\n", t1 ? "PASS" : "FAIL", t2 ? "PASS" : "FAIL", t3 ? "PASS" : "FAIL");
    remove(pa);
    remove(pb);
    SleepThread();
    return 0;
}

@PS2HomeDeveloper PS2HomeDeveloper closed this by deleting the head repository Nov 4, 2026
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

Successfully merging this pull request may close these issues.

1 participant