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

[Leif Lindholm <leif.lindholm@linaro.org>]

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.
>