Re: [PATCH] spi: bcm2835: Fix buffer overflow with CS able to go beyond limit.

On Sat, May 01, 2021 at 09:51:35PM +0200, Lukas Wunner wrote:

On Fri, Apr 23, 2021 at 05:20:55PM +0100, Mark Brown wrote:

quoted

Part of the issue here is that there has been some variation in how
num_chipselect is interpreted with regard to GPIO based chip selects
over time.  It *should* be redundant, I'm not clear why it's in the
generic bindings at all but that's lost to history AFAICT.

It seems num_chipselect is meant to be set to the maximum number of
*native* chipselects supported by the controller.  Which is overwritten
if GPIO chipselects are used.

This gets fun with the controllers that have for various reasons open
coded some or all of the GPIO chip select handling.

I failed to appreciate that when I changed num_chipselects for
spi-bcm2835.c with commit 571e31fa60b3.  That single line change
in the commit ought to be reverted.

And the kernel-doc ought to be amended because the crucial detail
that num_chipselect needs to be set to the maximum *native* chipselects
isn't mentioned anywhere.

Can you send patches for these please?

quoted

The best thing would be to have it not have a single array of chip
select specific data and instead store everything in the controller_data
that's there per-device.

Unfortunately that's non-trivial.  The slave-specific data is DMA-mapped.
It could be DMA-mapped in ->setup but there's no ->unsetup to DMA-unmap
the memory once the slave is removed.  Note that the slave could be removed
dynamically with a DT overlay, not just when the controller is unbound.

So we'd need a new ->unsetup hook at the very least to make this work.

There's the cleanup() callback which seems to fit?