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

devkitarm: file read data corruption #16

Closed
bgK opened this issue Feb 6, 2020 · 3 comments
Closed

devkitarm: file read data corruption #16

bgK opened this issue Feb 6, 2020 · 3 comments

Comments

@bgK
Copy link
Contributor

bgK commented Feb 6, 2020

While investigating this downstream bug, I've noticed an issue with a patch devkitPro does to newlib's fread function: https://bugs.scummvm.org/ticket/11342

The program does the following calls to trigger data corruption:

	char *buffer = (char *)malloc(20*1024);

	// Open a file for reading with buffered IO
	FILE *f = fopen("sdmc:/ScummVM/The Dig/DIG.LA1", "r");

	// Go somewhere in the file, unimportant
	fseek(f, 23800663, SEEK_SET);
	// Initialize the stdio read buffer
	fread(buffer, 1, 1, f);
	// Make a large read that enters devkitPro's fread patched code path.
	// After this, the stdio read buffer is out of sync with the stream position.
	fread(buffer, 1, 10096, f);
	// Seek back a bit in the file. The target seek position is inside the buffer from
	// stdio's perspective (but actually is not as the buffer is out of sync with the actual
	// stream position).
	fseek(f, 23810752, SEEK_SET);
	// Make a small read. The data is fetched from the stdio buffer. It is inconsistent with what is actually in the file.
	fread(buffer, 1, 4, f);

	fclose(f);

I suggest reverting this patch:

if (resid>BUFSIZ)

@fgsfdsfgs
Copy link

Can confirm I've encountered this bug on Switch, especially when reading large files like PK3 archives in Quake 3.

@mtheall
Copy link

mtheall commented May 4, 2020

I have tested with the fread.c file reverted to original newlib code and this sequence of events still reads the wrong data.

False alarm

@WinterMute
Copy link
Member

this is fixed with devkitARM r54 and devkitA64 r15

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

4 participants