tests/kernel/userspace: Remove tests for undocumented/legacy behavior

[andyross]

The test_{read,write}_other_stack() cases would launch a thread synchronously into the parent mem_domain and expect that memory access to the "foreign" thread would fail.

In fact that's not the documented behavior. Threads in the same mem_domain should have the same view of memory and SHOULD be able to access the stacks of other threads in the same domain. These are validating in exactly the wrong direction!

The only platforms on which this test can pass are legacy MPU implementations that handle stacks specially and not according to the domain mapping. MMU systems are configured to skip the tests, as they implement access correctly per docs.

Just remove the tests, there's no value here. See bug #70457 for dicsussion about how to address the inconsistent userspace stack behavior.

[nashif]

how does removing tests fix the problem? Obviously we have the functionality still there, it was tested, it was passing for some configurations. There need to be some documentation change somewhere or a note clarifying this...

[andyross]

Uh.... confused. I had a -1 on the patch that merged, and there seems to have been consensus that this was a better fix?

The tests are being removed because they're not testing a feature we document, and that we implement inconsistently across platforms. The linked bug tracks discussion on how to fix this for real.

[nashif]

The tests are being removed because they're not testing a feature we document, and that we implement inconsistently across platforms. The linked bug tracks discussion on how to fix this for real.

my point is that removing the tests ONLY does not change the behviour and the feature is still there, i.e. on MPUs we had until now, those tests actually apply and test something, so we could document this somewhere as a (positive?) sideeffect and not expected behaviour, the expected behaviour is (https://docs.zephyrproject.org/latest/kernel/usermode/overview.html#threat-model):

A user thread will by default have read/write access to its own stack buffer.
A user thread will never by default have access to user thread stacks that are not members of the same memory domain.
A user thread will never by default have access to thread stacks owned by a supervisor thread, or thread stacks used to handle system call privilege elevations, interrupts, or CPU exceptions.

what if someone was relying on this behaviour where thread do not have access to each other stacks in the same domain with existing MPU implementations?

[andyross]

what if someone was relying on this behaviour where thread do not have access to each other stacks in the same domain with existing MPU implementations?

That's actually the worst possible case, because that's a security flaw. Such an app would have a silent hole if you built it for a MMU platform. That's the core bug.

This is just a hygine patch: the existing code in the tree leaves these two test in place, implicitly promising the behavior, but then skips it on all but the old MPU platforms, using some logic I didn't like (because it involves testing against an unrelated API variant, which I found confusing). This was proposed as a cleaner fix, since the tests aren't validating a feature we want to support (precisely because of the security implications).

[fabiobaltieri]

needs a rebase

[aescolar]

@andyross please remember that this PR needs a rebase.

[aescolar]

@andyross please remember that this PR needs a rebase :)
I'm setting the dnm label so this PR stops cluttering the short merge list. Please remove the label once it has been rebased.

[andyross]

I'll close this one. Discussion on the related RFC will need to be sorted out first; there's some resistance to deprecating the older "MPU-style" stack behavior this is testing.