[riscv-CMOs:master] new issue: Feedback on the 1.0-rc2 specification #github #CMOs #risv

tech-cmo@lists.riscv.org Integration <tech-cmo@...>

[riscv-CMOs:master] New Issue Created by azuepke:
#44 Feedback on the 1.0-rc2 specification

Dear authors,

here's some feedback to the 1.0-rc2 specification from a microkernel developer's point of view. Thank's for the nice work so far!

  1. Do the CBO instructions affect instruction caches?

After reading the current specification, it is not clear to me (and probably other readers as well) if the CBO instructions target both data and instruction caches, or affect data caches only. From the wording of the document ("... or instruction fetch is permitted ..."), it seems that CBO instructions could be used to invalidate instruction caches as well. Also, a dedicated an IC IVAU (ARM) or ICBI (PPC) instruction is not specified either (probably redundant to FENCE.I?). This should be clarified in the document.

  1. Mandatory write permissions for CBO.INVAL

My second concern is about potential security issues in CBO.INVAL. The specification should require mandatory write permissions in the TLBs for CBO.INVAL.

The problem is as follows: Assume a shared memory segment (SHM) between different processes. A writer process writes data to the SHM, and multiple readers read from the SHM. The readers just have read permissions to access the SHM. Now, a reader performs a CBO.INVAL on data in the SHM, as the specification only requires that: > A cache-block management instruction is permitted to access the specified cache block whenever a load instruction, store instruction, or instruction fetch is permitted to access the corresponding physical addresses.

However, this allows the reader to alter data in the SHM if the data is dirty in the caches. Requiring write permissions in the TLB for CBO.INVAL to succeed instead of just any of RWX permissions would fix this issue. This check should also be performed in the CBIE=01 setting when CBO.INVAL just flushes data. Always checking the write permissions makes CBO.INVAL similar to CBO.ZERO. In any case, for exceptions, reporting CBO.INVAL (and CBO.ZERO) as store page faults is fine.

Note that "being careful" and "disabling CBO.INVAL for user space and only use cache invalidation in supervisor mode" does not help here. Consider an example where the OS kernel invalidates cachelines in a mapping that is known to be writable on one core, and in parallel another core maps a different read-only page to the same address. Then the first core will continue invalidating the now read-only page. I know that this is not a realistic use case, but think of the Dirty COW exploit on Linux or VM escapes that could be prevented by checking for write permissions.

Also note that the same issue exist for the DCIMVAC instruction in the ARM v7 specification. This was fixed in ARM v8 so that DC IVAC now requires write permissions to the memory. The related DCBI instruction in PowerPC also requires write permissions. And both architectures report permission errors as failing store exceptions.

  1. CBO.CLEAN and CBO.FLUSH should report "load"-type exceptions

Related to the previous one: if CBO.CLEAN and CBO.FLUSH require just read permissions to work, then they should report a "load"-type pagefault / access fault in exceptions as well.

This helps the paging algorithm in the OS kernel. when an application performs a CBO.FLUSH to a currently unmapped (paged out) address, it would be reported as a read fault and not a write fault, so the OS could install the mapping with just read permissions for the CBO.FLUSH to succeed.

Note that this is consistent with the mental model that CBO.CLEAN and CBO.FLUSH do not alter any data, and therefore require only read permission. Also, PowerPC and ARM handle these as reads rather than writes.

I hope it's not too late to address the last two points in the specification.

All the best Alex