Compartment configuration files

[0152la]

Now can specify compartment entry points via a toml file of the name <binary>.comp, where <binary> is the name of the ELF binary to be compartmentalize, and the associated configuration file should reside in the same folder as the compartment binary.

The format of the toml file is:

• each entry point is defined as a toml table, with the table name being the entry function name in the compartment binary;
• entry args_count is of type number, and says how many arguments we expect to pass to the entry function;
• entry args_type is an array and enumerates the types to be passed to the function.

This comes with additional internal functionality to convert between types (the conversion is done manually, i.e., new types need to be added to the code, rather than being dynamically handled).

Additional tests added, and reworked how tests in CMake are done, to better handle argument support.

[0152la]

I wanted to get this PR up since I think I underestimated it's scope and it's growing more than I'd like. There's a bit of cleanup I saw just as I was writing the commit message, such as maybe args_count in the config file is unneeded (already have the number of types as the length of the array), and some tests have been superseded by multiple entry points in args_simple. However, I think that's beside the main points of this PR, and I might fold them in as we progress with this one, or relegate them to a future PR (I also have some odd building issues to go over - shouldn't affect this PR, but buildbot validation will help my confidence).

[ltratt]

This comes with additional internal functionality to convert between types

I'm not sure I understood why this is necessary. Any hints?

[0152la]

I'm not sure I understood why this is necessary. Any hints?

This refers to the casting code for arguments [1], to more tightly pack them in memory. I added this since arguments go to the manager as char * (as part of argv), but we pass the expected type internally.

As to why it's necessary, at least at this point there is no pointer support to be able to straight pass char *. But even with that, I think passing those char * directly would again lead to a double malloc situation as we had before (insert question mark for myself).

[1] https://github.com/capablevms/CHERI-ELF-comp/pull/7/files#diff-adfde91070db01742cb89ede1d5f00ff6ca15a8f01b6606c1bae07ee95b132ddR313-R345

[ltratt]

As to why it's necessary, at least at this point there is no pointer support to be able to straight pass char . But even with that, I think passing those char directly would again lead to a double malloc situation as we had before (insert question mark for myself).

Is there an element of premature optimisation here? [I ask this a bit naively!]

[0152la]

I think this could be greatly simplified, without harming the example, by passing all arguments in 64-bit slots. (That's effectively what AAPCS64 does for the supported types, since they're all passed in X registers or 64-bit stack slots.) That would make the parsed_args array homogeneous, would avoid some complex assembly sequences, and would make the void** type more accurate.

As I've said in the comments, that void ** is meant to be void *, to avoid the double malloc approach in the previous implementation, and to make better use of space (which I think also addresses @ltratt's question above, as casting the arguments passed to the manager as strings to their intended types should pack them more tightly in the args memory, thus wasting less space. We could just use 64 bits per argument, however; as @jacobbramley says, it would simplify things.

[ltratt]

We could just use 64 bits per argument

It does seem like that might be a worthwhile simplification.

[ltratt]

I'm fine with this so it's @jacobbramley's comments left.

[0152la]

It does seem like that might be a worthwhile simplification.

I was thinking about this (while also working on adding an extra test to do with arguments being set near unmapped / inaccessible memory regions, which has proven a bit difficult (but would also be potentially unneeded if this optimization happens, but might as well finish it now)). I think I'd like to have this PR as is, as having the code in the history would be useful. Though I guess there's no need to squash to a single commit, and could have two commits, with this dynamic memory allocation, and one with static 64 bits.

Which also reminded me that this code won't work if we want to pass capabilities as parameters, since they are 128 bits. This could be fixed by using c registers in the transition code, and 128 bit static slots (if we go that way), so it won't be too much engineering effort, but more a design question.

Nevertheless, for this PR, I'd like to finalise adding that one extra test I've been working on, and merge it as is (once Jacob's comments are done), if that's fine.

[ltratt]

Nevertheless, for this PR, I'd like to finalise adding that one extra test I've been working on, and merge it as is

Let's do that. Since Jacob is away, I think we should merge this PR when you've done the above.

[0152la]

As discussed earlier, implementing the new test did show an error when the arguments are stored in a memory region near an unmapped region (haven't tested with private regions yet). To fix this, I'd like to go implement the idea of just having 64-bit wide slots for arguments. That will remove some code and simplify some things.

However, I'd like to keep the current code in the history, and have another commit on top of it once the new implementation is done. As such, could I squash these commits together now, to make it easier to keep track of what I want to keep?

[ltratt]

However, I'd like to keep the current code in the history, and have another commit on top of it once the new implementation is done. As such, could I squash these commits together now, to make it easier to keep track of what I want to keep?

Please do.

[0152la]

Ready for review (changes detailed in body of https://github.com/capablevms/CHERI-ELF-comp/pull/7/commits/e02bc0db9c68a3eb1ba404a161f5f2af52efae5c).

[ltratt]

Please squash.

[0152la]

Squashed

[ltratt]

bors r+

[bors[bot]]

Build failed:

• buildbot/ci-builder

[0152la]

bors try

[bors[bot]]

try

Build failed:

• buildbot/ci-builder

[0152la]

bors try

[bors[bot]]

try

Build failed:

• buildbot/ci-builder

[0152la]

bors try

[bors[bot]]

try

Build failed:

• buildbot/ci-builder

[0152la]

bors try

[bors[bot]]

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• buildbot/ci-builder

[0152la]

Ready for review.

[ltratt]

Please squash.

[0152la]

Squashed

[ltratt]

bors r+

[bors[bot]]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• buildbot/ci-builder