Repository navigation
fix(libcglue): fix descriptor table handling in dup, dup2 and open - #956
Closed
PS2HomeDeveloper wants to merge 5 commits into
Closed
PS2HomeDeveloper wants to merge 5 commits into
PS2HomeDeveloper wants to merge 5 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This fixes three bugs in the file descriptor table of libcglue
(
ee/libcglue/src/fdman.canddup2()inglue.c).1.
dup()anddup2()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 equalsthe descriptor number, instead of the entry the descriptor actually uses
(
__descriptormap[fd]). Those are the same only whenfdwas notcreated by a previous
dup(). Fordup(dup(fd))the new descriptorpointed 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 numberiwith pool entryi. If the original descriptor was closed while aduplicate was still open, that pool entry was still in use, and the next
open()filled it with the new file, so the duplicate suddenly referredto 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 rejectednewfd < 0. Withnewfd >= __FILENO_MAX(64) itread
__descriptormap[newfd]and then wrote the new entry there, outsideof the table. It now fails with
EBADF, and the same check was added in__fdman_get_dup2_descriptor().Impact
dup()/dup2()on a duplicate now work, and closing the originaldescriptor 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 withB) and checks:read()ondup(dup(fd))returns the file's data.dup(fd)andclose(fd), opening the second file must notchange what the duplicate reads.
dup2(fd, 64)must fail withEBADF.The same program was built twice, once without and once with this change.
The
dup()anddup2()code was the same in both builds apart fromthis change (the first build used libcglue from master plus two of my
own earlier patches, #950 and #954, which do not touch this code).
duplicate read the data of the other file), T3 failed (
dup2returned64 instead of failing).
Result before the change
Result after the change
Test program (main.c)