Correct memory size calculation for modules on linux#255
Conversation
|
One thought that is only now occuring to me. I've made the assumption that I've no idea how much of a problem this is in practice, maybe this is fine. Otherwise I suggest we parse |
loader/utility.cpp
Outdated
| */ | ||
| lib.memorySize = PAGE_ALIGN_UP(hdr.p_filesz); | ||
| break; | ||
| lib.memorySize += hdr.p_memsz; |
There was a problem hiding this comment.
This doesn't account for space between each segment - do we just want whatever the max is? The biggest p_vaddr + p_memsz
There was a problem hiding this comment.
Just read your comment about this after reviewing, should only significantly change this if there's a requirement to do so
There was a problem hiding this comment.
This doesn't account for space between each segment - do we just want whatever the max is? The biggest p_vaddr + p_memsz
Given this was the original (the PR) solution and it led to a crash, I can only assume we cannot rely on p_vaddr
There was a problem hiding this comment.
Alright, bit tired today and didn't fully process your message. But now I have lol. Anyways I think this is how we should be going about it.
2 participants
Saw the conversation happening on discord about this specific change, as well as #253 and #251.
While the solution given isn't correct, @azzyr remains right in their statement that we're just lucky that
.rodatahappens to be found within the aligned page's size. I'm not at all familiar with how modules are loaded on linux, however a quick read of the manual on elf does tell us thatPT_LOADsegments will have as final size in memoryp_memszwhich can be 0 (forPT_LOADsegments that end up not taking any place in memory).I'm not quite sure why the #251 PR as well as the original code both stubbordingly limit their calculation to one
PT_LOADsegment without really explaining the reasoning behind. The module size will at least be the addition of allPT_LOAD'sp_memsz, so let's do just that.