Spi/MvSpiDxe likely NullPtrException

Spi/MvSpiDxe likely NullPtrException

[methavanitpong.pipat]

Hi,

MvSpiDxe.c:MvSpiTransfer() attempts load data into DataInPtr, which is
a possibly NULL pointer. 

    https://github.com/tianocore/edk2-platforms/blob/master/Platform/Marvell/Drivers/Spi/MvSpiDxe.c#L228
    ///////////// SNIP /////////////
    for (Iterator = 0; Iterator < SPI_TIMEOUT; Iterator++) {
      if (MmioRead32 (SpiRegBase + SPI_INT_CAUSE_REG)) {
        *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
    ////////////////////////////////

One example that DataInPtr == NULL is in MvSpiDxe.c:MvReadWrite() in 
the first MvSpiTransfer() call. 

    https://github.com/tianocore/edk2-platforms/blob/master/Platform/Marvell/Drivers/Spi/MvSpiDxe.c#L276
    ///////////// SNIP /////////////
    Status = MvSpiTransfer (This, Slave, CmdSize, Cmd, NULL, SPI_TRANSFER_BEGIN);
    if (EFI_ERROR (Status)) {
      Print (L"Spi Transfer Error\n");
      return EFI_DEVICE_ERROR;
    }

    Status = MvSpiTransfer (This, Slave, DataSize, DataOut, DataIn, SPI_TRANSFER_END);
    if (EFI_ERROR (Status)) {
      Print (L"Spi Transfer Error\n");
      return EFI_DEVICE_ERROR;
    }
    ////////////////////////////////

It should store data from SPI_DATA_IN_REG in a temporary variable first, 
then passes the data into DataInPtr if DataInPtr != NULL. 
--
Pipat Methavanitpong
Software Developer, S-Project 3
Socionext Inc.

Re: [edk2-platforms] Spi/MvSpiDxe likely NullPtrException

[Leif Lindholm]

Hi Pipat,

It is useful to prefix emails regarding edk2-platforms with
[edk2-platforms] on the subject line.

+Jan Dąbroś <jsd@semihalf.com> (original author, a fact that has been
lost in the migration from OpenPlatformPkg.
also +Marcin Wojtas <mw@semihalf.com>, Marvell platform owner.
(Neither of which are obvious from looking at the source tree.)

/
    Leif

On Wed, Sep 13, 2017 at 12:20:33AM +0000, methavanitpong.pipat@socionext.com wrote:
> Hi,
> 
> MvSpiDxe.c:MvSpiTransfer() attempts load data into DataInPtr, which is
> a possibly NULL pointer. 
> 
>     https://github.com/tianocore/edk2-platforms/blob/master/Platform/Marvell/Drivers/Spi/MvSpiDxe.c#L228
>     ///////////// SNIP /////////////
>     for (Iterator = 0; Iterator < SPI_TIMEOUT; Iterator++) {
>       if (MmioRead32 (SpiRegBase + SPI_INT_CAUSE_REG)) {
>         *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
>     ////////////////////////////////
> 
> One example that DataInPtr == NULL is in MvSpiDxe.c:MvReadWrite() in 
> the first MvSpiTransfer() call. 
> 
>     https://github.com/tianocore/edk2-platforms/blob/master/Platform/Marvell/Drivers/Spi/MvSpiDxe.c#L276
>     ///////////// SNIP /////////////
>     Status = MvSpiTransfer (This, Slave, CmdSize, Cmd, NULL, SPI_TRANSFER_BEGIN);
>     if (EFI_ERROR (Status)) {
>       Print (L"Spi Transfer Error\n");
>       return EFI_DEVICE_ERROR;
>     }
> 
>     Status = MvSpiTransfer (This, Slave, DataSize, DataOut, DataIn, SPI_TRANSFER_END);
>     if (EFI_ERROR (Status)) {
>       Print (L"Spi Transfer Error\n");
>       return EFI_DEVICE_ERROR;
>     }
>     ////////////////////////////////
> 
> It should store data from SPI_DATA_IN_REG in a temporary variable first, 
> then passes the data into DataInPtr if DataInPtr != NULL. 
> --
> Pipat Methavanitpong
> Software Developer, S-Project 3
> Socionext Inc.
>