[ipc] Verify if shared buffer length is a power of 2

[alexandruradovici]

Pull Request Overview

This pull request adds a verification for the ipc shared buffer length if it is 0 or a power of two.

As each shared buffer should be mapped as an MPU region. at least for ARM architectures, the length of these buffers should be a power of 2 or zero (if the buffer is unallowed). At least this is my understanding, but I do not have a lot of experience working with the MPU.

Without this verification, all the ipc functions in user space return success values, but when the service tries to access the shared buffer, the process faults. I am not sure that this verification solves completely the problem, but at least diminishes the possibility of errors as it rejects buffers that do not have a suitable length.

I tried adding this verification in the schedule_upcall function when the kernel tries to map the new MPU region. It turns out that the MPU returns None if the requested region already exists or overlaps with another region, thus making it impossible to distinguish between an error and a region that already exists. Adding the MPU region within allow_readwrite is not a solution as the buffer will be unallowed and allowed again by the process. The second time the same buffer is allowed, the add_mpu_region function will return None as the region is already there.

A better solution would probably be to verify whether the shared buffer is accessible by the process before scheduling the upcall. This implies adding a function like has_access_to to Process. Considering that this approach is not a quick fix and requires a larger discussion, I think the solution of checking the length of the buffer is simple and good enough for now.

Testing Strategy

This pull request was tested using a Micro:bit.

TODO or Help Wanted

I might have misunderstood the requirements for the buffer length.

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[hudson-ayers]

I appreciate that this is meant as a quick fix, but it sounds to me like the right fix is to modify the MPU interface to actually return errors instead of letting None indicate multiple different failure modes. It seems possible that some MPUs will not have power-of-two restrictions, or will have more strict restrictions which this will not cover.

[ppannuto]

+1 to Hudson's comment. Even on ARM, it's more nuanced than just power-of-two with subregions (basically it's power of 2 +/- some number of 1/8th of the power-of-two region size). MPUs seem to be complicated with higher variance across implementations than would be ideal :/

[bradjc]

Closing this, but noted on #1993. We either need an entirely new implementation of IPC, or to have the MPU try to allocate the region and return an error if that fails. Hardcoding a power of two size limit is wrong on RISC-V, for example.