memory_maps: move ia2_threads_metadata global definition to INIT_RUNTIME to have a once-mapped definition
#619
Conversation
kkysen
changed the title
INIT_RUNTIME to have a once-mapped definitionia2_threads_metadata global definition to INIT_RUNTIME to have a once-mapped definition
kkysen
force-pushed
the
kkysen/memory-maps-move-global-to-INIT_RUNTIME
branch
from
2f746d8 to
f3107c2
Compare
kkysen
force-pushed
the
kkysen/memory-maps-move-global-to-INIT_RUNTIME
branch
from
f3107c2 to
e5dcc29
Compare
kkysen
force-pushed
the
kkysen/memory-maps-move-global-to-INIT_RUNTIME
branch
from
e5dcc29 to
1db4ac5
Compare
|
I only see I do agree that we only want a single copy of that variable in the address space to make things easier to understand, but in my opinion the way to do that is to make libia2.a a shared library. Stuffing more variables into a macro makes things harder to understand if anything, so if this PR isn't solving a specific problem I don't think we should merge it. |
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
I do agree that we only want a single copy of that variable in the address space to make things easier to understand, but in my opinion the way to do that is to make libia2.a a shared library. Stuffing more variables into a macro makes things harder to understand if anything, so if this PR isn't solving a specific problem I don't think we should merge it.
Oh, sure, we could definitely make libia2 a shared library instead. I just thought we decided that wasn't worth it. Happy to change.
It definitely is lower priority relative to closing the PRs we have open, the tracer stuff and compartmentalizing glibc (especially since the latter two are big holes in the whole sandbox), but I think it would be worthwhile to make things easier to understand/audit. We should open an issue for this and first enumerate everything that libia2.a and the rewriter-generated sources provide. Then figure out what we can put into a shared library and what we'd need to link into each DSO (and find a reasonably ergonomic way to do that). |
… have to be public again
This allows `ia2_internal.h` to reference things defined in `ia2.h`. Moving this is mostly straightforward, except the `#define _GNU_SOURCE` had to be moved to the top of `ia2.h`.
…UNTIME` to have a once-mapped definition
Without this, `liblibia2.a` contains the `ia2_threads_metadata`,
which is then linked into each compartment's `*.so`,
causing each compartment to reference a different version of the global,
causing lots of errors and segfaults.
This also requires moving the type and other definitions
from `memory_maps.{h,c}` to `ia2.h`.
…d `ia2_internal.h` This lets us move all non-public definitions to `ia2_internal.h` instead of the public `ia2.h`, namely the memory maps types. Moreover, `ia2.h` can go back to including `ia2_internal.h` at the top instead of the bottom, since the definitions it needs from `ia2.h` are now in `ia2_common.h`.
kkysen
force-pushed
the
kkysen/memory-maps-move-global-to-INIT_RUNTIME
branch
from
1db4ac5 to
100132e
Compare
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
We should open an issue for this and first enumerate everything that libia2.a and the rewriter-generated sources provide. Then figure out what we can put into a shared library and what we'd need to link into each DSO (and find a reasonably ergonomic way to do that).
I opened #633 for that.
Without this,
liblibia2.acontains theia2_threads_metadata, which is then linked into each compartment's*.so, causing each compartment to reference a different version of the global, causing lots of errors and segfaults.This also requires moving the type and other definitions from
memory_maps.{h,c}toia2.h.This is split off of #585 as it's really a separate issue that was surfaced in trying to implement #585. Also, this is now rebased after #561 was merged, as #585 was waiting on #561 to merge first for a cleaner implementation.