| Message ID | 20220829080531.29681-1-johan+linaro@kernel.org |
|---|---|
| Headers | show |
| Series | misc: fastrpc: fix memory corruption | expand |
Hi Johan, On 29/08/2022 09:05, Johan Hovold wrote: > The fastrpc driver uses a fixed-sized array to store its sessions but > missing and broken sanity checks could lead to memory beyond the array > being corrupted. > > This specifically happens on SC8280XP platforms that use 14 sessions for > the compute DSP. > Thanks for doing this. I see that we hit this issue once again, and the way we are fixing it is not really scalable. We should really get rid of FASTRPC_MAX_SESSIONS. We should allocate the sessions dynamically based in the child node count and qcom,nsessions. thanks, Srini > These are all needed for 6.0. > > Johan > > > Johan Hovold (3): > misc: fastrpc: fix memory corruption on probe > misc: fastrpc: fix memory corruption on open > misc: fastrpc: increase maximum session count > > drivers/misc/fastrpc.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) >
On Fri, Sep 02, 2022 at 11:02:35AM +0100, Srinivas Kandagatla wrote: > Hi Johan, > > On 29/08/2022 09:05, Johan Hovold wrote: > > The fastrpc driver uses a fixed-sized array to store its sessions but > > missing and broken sanity checks could lead to memory beyond the array > > being corrupted. > > > > This specifically happens on SC8280XP platforms that use 14 sessions for > > the compute DSP. > > > Thanks for doing this. > > I see that we hit this issue once again, and the way we are fixing it is > not really scalable. We should really get rid of FASTRPC_MAX_SESSIONS. Yeah, I was a bit surprised to find that the underlying bugs (i.e. the incomplete sanity checks) weren't fixed the last time this memory corruption was reported: https://lore.kernel.org/all/1632123274-32054-1-git-send-email-jeyr@codeaurora.org/ > We should allocate the sessions dynamically based in the child node > count and qcom,nsessions. That sounds like it would be an improvement. But at least now you'll get an error message during probe rather than silent memory corruption when bringing up a new SoC that needs more than the current maximum number of sessions. Johan