Implementation issues in LinuxSpiDriver

[celskeggs]

F´ Version	latest
Affected Component	Drv.LinuxSpiDriver

---

Problem Description

I've been reviewing the LinuxSpiDriver implementation so that I can create an equivalent F Prime component for another platform. During this review, I noticed two issues that we should consider fixing.

First, this code has a potential out of bounds memory access violation:

        tr.tx_buf = reinterpret_cast<__u64>(writeBuffer.getData());
        tr.rx_buf = reinterpret_cast<__u64>(readBuffer.getData());
        tr.len = writeBuffer.getSize();

The spi_ioc_transfer struct's len field provides the length of both tx_buf and rx_buf. If the caller passes a readBuffer with a shorter length than the writeBuffer, the kernel may write past the end of rx_buf, potentially corrupting unrelated memory. There should be a FW_ASSERT here to ensure that the sizes of the buffers are the same.

Second, the open function unnecessarily reads back the settings that it writes, without verifying them:

        ret = ioctl(fd, SPI_IOC_WR_MODE, &mode);
        if (ret == -1) {
            this->log_WARNING_HI_SPI_ConfigError(device,select,ret);
            return false;
        }

        ret = ioctl(fd, SPI_IOC_RD_MODE, &mode);
        if (ret == -1) {
            this->log_WARNING_HI_SPI_ConfigError(device,select,ret);
            return false;
        }

SPI_IOC_WR_MODE updates the setting for MODE in the Linux kernel SPI driver. SPI_IOC_RD_MODE reads that setting back out. It is not clear why the setting is read back out, because it is not actually verified.

If the purpose of this code is to verify that the kernel accepted the provided value of mode, then there needs to be a check on the value of mode provided by SPI_IOC_RD_MODE. If not, it is not clear why this second ioctl occurs, and it should either be clarified in a comment or removed.

I don't currently have access to any SPI peripherals for Linux devices, so I'm unable to make any fixes myself. I also have no need for these fixes at this time. But I want to capture these two issues so that they can be fixed in the future. Developers of SPI drivers for other platforms are likely to look at and copy this code, so it would be useful for this code to be fully correct and not misleading.