copy_handler_table_to_cage: 1. Fix incorrect argument usage 2. Overwrite target cage table.#1113
copy_handler_table_to_cage: 1. Fix incorrect argument usage 2. Overwrite target cage table.#1113stupendoussuperpowers wants to merge 3 commits intomainfrom
copy_handler_table_to_cage: 1. Fix incorrect argument usage 2. Overwrite target cage table.#1113Conversation
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
CI tests pass on this so not a breaking change, we should discuss whether this change is acceptable since the fs-tee-grate relies on this. |
Yaxuan-w
left a comment
There was a problem hiding this comment.
The change looks good to me. Can you also put a unit test for this?
What kind of test should I add? A grate test? |
Yes. |
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Added the test, re-requesting review. |
rennergade
left a comment
There was a problem hiding this comment.
I think this works. Want to get @JustinCappos approval on this before merge.
The functionality of copying a handler table wasn't well thought out. I was thinking mostly of needing to support fork and perhaps some filter grates. I wanted to make something simple and hard to mess up. Perhaps something which takes a starting call number and ending call number to map makes sense? Maybe this also needs to apply a change so that all system call number offsets start at their location - the starting call number + some provided argument. (Obviously integer over/underflows need to be carefully checked here.) As for deleting or merging into the existing table, this also could be an option. This all is starting to seem a bit more complex. So we need really clear testing if we go this route. |
Adds 1 fix and 1 change:
Fix:
copy_handler_table_to_cagein 3i mismatched the argument orders in glibc, fixed that, and updated the usage in fork_syscall to match this.Change (needs discussion if this passes CI):
Right now copy handler table to cage merges source into target cage's handler table, for the fs-tee-grate, it's useful to have a functionality where we completely overwrite the target handler table with that of source's. This essentially let's us rewrite a cage's parent cage as far as the inheritance of handler tables is concerned.
As as example -
If these are run linearly, 3 inherits all handlers on 2, if we have the ability to overrwrite handler tables, we can copy 1 into 3 and 3 now for all routing purposes behaves as if it's a direct child of 1.
My questions are:
Was the decision to merge these two tables deliberate, and if so, for which use case?
Does this change break any key assumptions we have about this function?