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

[ELFLOADER] Fixed align issue (may help #1057) #1058

Merged
merged 1 commit into from
Nov 12, 2023
Merged

Conversation

ksco
Copy link
Collaborator

@ksco ksco commented Nov 12, 2023

for #1057

@ptitSeb
Copy link
Owner

ptitSeb commented Nov 12, 2023

There is no mention in linux docs of mmap that lenght should be aligned to pagesize. Only addr and offset need to be. Did you test your change extensively? because elfloader is really sensitive! Does this change fix the ticket?

@ksco
Copy link
Collaborator Author

ksco commented Nov 12, 2023

IIUC, at line 258, we're calculating an aligned size for current fragment which will be used to map memory for the fragment. So we should use the head->multiblocks[n].paddr instead of e->p_paddr.

Before this fix, wine will crash at line 312 inside fread as we're not mapping enough memory for it.

Some logs after the fix:

Mmap64 for (@0x7f00000000 0x2bc500) for elf "/home/debian/wine-8.20/bin/../lib/wine/x86_64-unix/ntdll.so" returned 0x3fd6c54000 instead
Using emulated /home/debian/wine-8.20/bin/../lib/wine/x86_64-unix/ntdll.so
Mmap64 for (@0x7f01000000 0x1bbc8) for elf "/lib/x86_64-linux-gnu/libunwind.so.8" returned 0x3fd6c38000 instead
Using emulated /lib/x86_64-linux-gnu/libunwind.so.8

@ksco
Copy link
Collaborator Author

ksco commented Nov 12, 2023

Does this change fix the ticket?

According to the reporter, yes, and it fixed wine for me as well.

@ptitSeb ptitSeb merged commit 5d2ae4a into ptitSeb:main Nov 12, 2023
28 checks passed
@ksco ksco deleted the debug branch November 12, 2023 14:22
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.

2 participants