[RFC PATCH 0/1] arm64: cacheinfo: Avoid out-of-bounds write when DT info is incorrect

[Radu Rendec]

Hi all,

I found an out-of-bounds write bug in the arm64 implementation of
populate_cache_leaves() and I'm trying to fix it. The reason why this
is an RFC is that I'm not entirely sure these changes are sufficient.

The problem is described in detail in the patch itself, so I won't
repeat it here. The gist of it is that on arm64 boards where the device
tree describes the cache structure, the number of cache leaves that
comes out of fetch_cache_info() and is used to size the cacheinfo array
can be smaller than the actual number of leaves. In that case,
populate_cache_leaves() writes past the cacheinfo array bounds.

The way I fixed it, the code doesn't change too much and doesn't look
ugly but it's still possible to write past the array bounds if the last
populated level is a separate data/instruction cache. But I'm not sure
if this is a real-world scenario, so this is one of the areas where I'm
looking for feedback. For example, the DT may define a single unified
level (so levels=1, leaves=1) but in fact L1 is a split D/I cache. Or
the DT defines multiple levels, the last one being a unified cache, but
in reality the last level is a split D/I cache. I believe the latter
scenario is very unlikely since typically only L1 is a split D/I cache.

The other thing that doesn't look right is that init_of_cache_level()
bumps the level at each iteration to whatever the node corresponding to
that level has in the `cache-level` property, but that way one or more
levels can be skipped without accounting any leaves for them. This is
exactly what happens in my case (the Renesas R-Car S4 board, described
in arch/arm64/boot/dts/renesas/r8a779f0.dtsi). In this case, even if
populate_cache_leaves() is fixed, the cache information in the kernel
will be incomplete. Shouldn't init_of_cache_level() return -EINVAL also
when a level is skipped altogether?

Last, and also related to the previous question, is a device tree
definition that skips a cache level correct? Or, in other words, should
the definition in r8a779f0.dtsi be fixed?

Thanks!

Radu Rendec (1):
  arm64: cacheinfo: Avoid out-of-bounds write when DT info is incorrect

 arch/arm64/kernel/cacheinfo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.47.1