• David Lechner's avatar
    spi: axi-spi-engine: move msg state to new struct · 7f970ecb
    David Lechner authored
    This moves the message state in the AXI SPI Engine driver to a new
    struct spi_engine_msg_state.
    
    Previously, the driver state contained various pointers that pointed
    to memory owned by a struct spi_message. However, it did not set any of
    these pointers to NULL when a message was completed. This could lead to
    use after free bugs.
    
    Example of how this could happen:
    1. SPI core calls into spi_engine_transfer_one_message() with msg1.
    2. Assume something was misconfigured and spi_engine_tx_next() is not
       called enough times in interrupt callbacks for msg1 such that
       spi_engine->tx_xfer is never set to NULL before the msg1 completes.
    3. SYNC interrupt is received and spi_finalize_current_message() is
       called for msg1. spi_engine->msg is set to NULL but no other
       message-specific state is reset.
    4. Caller that sent msg1 is notified of the completion and frees msg1
       and the associated xfers and tx/rx buffers.
    4. SPI core calls into spi_engine_transfer_one_message() with msg2.
    5. When spi_engine_tx_next() is called for msg2, spi_engine->tx_xfer is
       still be pointing to an xfer from msg1, which was already freed.
       spi_engine_xfer_next() tries to access xfer->transfer_list of one
       of the freed xfers and we get a segfault or undefined behavior.
    
    To avoid issues like this, instead of putting per-message state in the
    driver state struct, we can make use of the struct spi_message::state
    field to store a pointer to a new struct spi_engine_msg_state. This way,
    all of the state that belongs to specific message stays with that
    message and we don't have to remember to manually reset all aspects of
    the message state when a message is completed. Rather, a new state is
    allocated for each message.
    
    Most of the changes are just renames where the state is accessed. One
    place where this wasn't straightforward was the sync_id member. This
    has been changed to use ida_alloc_range() since we needed to separate
    the per-message sync_id from the per-controller next available sync_id.
    Signed-off-by: default avatarDavid Lechner <dlechner@baylibre.com>
    Link: https://lore.kernel.org/r/20231117-axi-spi-engine-series-1-v1-9-cc59db999b87@baylibre.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
    7f970ecb
spi-axi-spi-engine.c 15.4 KB