Fix address translation for copy_data_between_cages

[stupendoussuperpowers]

This addresses #581.

Fix works by:

• Adding a TRANSLATE_UADDR_TO_HOST(uaddr, cageid) macro.
• Change check_addr to subtract vmmap.base_address before passing this to check_addr_mapping.
• Additionally, add implementation for the sc_convert_uaddr_to_host. Currently this function doesnt do anything. This is not required for this fix, can be removed if this is not needed in the long run.

My understanding is:

That within glibc, we only have access to __lind_base and __lind_cageid for the current/calling cage. i.e. when a grate calls copy_data_between_cages it is only capable of converting between uaddr and host for this specific __lind_cageid. The macro specified above assumes that if the cageid matches the __lind_cageid, it needs to append the base memory, otherwise it can ignore it.

If there is a cleaner approach to doing this translation, please let me know.

[stupendoussuperpowers]

Adding @Yaxuan-w @ssannkkallpp and @rennergade for comments/reviews.

[rennergade]

1. Failing the linter, so use cargo fmt
2. It seems to be failing tests specific to pthread. I know @ssannkkallpp looked into that a bit when adding the translation stuff maybe he has some ideas.

[stupendoussuperpowers]

I don't think the thread cases are failing because of any of the address translation changes. They seem to panic because the get_cage(cageid: 1) call returns None when called by the lind_signal_init function for threadid=2.

Interestingly, if I add a random println! for CAGE_MAP[1] in the get_cage function, it returns the correct cage as it is supposed to and the program runs as expected. This leads me to believe this might be a race condition of some sort.

@ssannkkallpp if you've encountered something similar please shed some light!

[stupendoussuperpowers]

I believe this is some sort of race condition given that the second test run succeeded for one of the previously failing thread.c case without having made any changes.

Looking into the mutex.c failure first.

[rennergade]

what is the different between this and the above function translate_ptr_to_host

[stupendoussuperpowers]

Takes an addr instead of a pointer. Also since copy_data_between_cages has no way to know which one of src or dest needs to converted, we also need to pass down the cageid info for it.

In essence it does the same thing (__lind_base + input) but only under specific conditions and not for a pointer.

[rennergade]

Oh I see that makes sense. Do we have to handle a NULL condition here?

[stupendoussuperpowers]

Don't think so, I believe it's handled at a later stage for copy_data, but will run some tests

[stupendoussuperpowers]

Tests are passing now, my new implementation of sc_convert_uaddr_to_host was converting 0 instead of ignoring it as NULL.

However,

The current implementation of this function does nothing, and is currently used in a handful of places -

copy_data_between_cages:

./threei/src/threei.rs:873:            let host_src_try = sc_convert_uaddr_to_host(srcaddr, srccage, thiscage);
./threei/src/threei.rs:906:    let host_src_addr = sc_convert_uaddr_to_host(srcaddr, srccage, thiscage);
./threei/src/threei.rs:907:    let host_dest_addr = sc_convert_uaddr_to_host(destaddr, destcage, thiscage);

futex_syscall:

./rawposix/src/fs_calls.rs:289:    let uaddr = sc_convert_uaddr_to_host(uaddr_arg, uaddr_cageid, cageid);
./rawposix/src/fs_calls.rs:292:    let val2 = sc_convert_uaddr_to_host(val2_arg, val2_cageid, cageid);
./rawposix/src/fs_calls.rs:293:    let uaddr2 = sc_convert_uaddr_to_host(uaddr2_arg, uaddr2_cageid, cageid);
./rawposix/src/fs_calls.rs:4220:    let buf = sc_convert_uaddr_to_host(buf_arg, buf_arg_cageid, cageid);

I believe these are unnecessary now that address translations occur in glibc. Should these be removed? (CC: @Yaxuan-w & @rennergade)

[stupendoussuperpowers]

Filed a PR to clean up remaining uses #589

[Yaxuan-w]

Great job! Could you also add unit test cases for this?

[stupendoussuperpowers]

Added comments.

Unit tests for grates (including a test for copy_data) are being added as a part of PR #590, it needs to wait for PR #571 to be merged first though, otherwise a lot of these tests will fail due to the issue with multiple syscall handler registrations.

[rennergade]

Ok looks good. Approved.