linux-kernel - Re: [PATCH 1/1] dmaengine: pl330: Check if the DMA descriptor is NULL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 20 Jul 2014 19:58:04 +0200
From:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
To:	Lars-Peter Clausen <lars@...afoo.de>,
	Dan Williams <dan.j.williams@...el.com>
CC:	Vinod Koul <vinod.koul@...el.com>, dmaengine@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] dmaengine: pl330: Check if the DMA descriptor is
 NULL

Hello Lars-Peter,

On 07/20/2014 04:18 PM, Lars-Peter Clausen wrote:
> On 07/19/2014 03:21 AM, Javier Martinez Canillas wrote:
>> Commit 6079d38 ("dmaengine: pl330: Remove useless xfer_cb indirection")
>> removed the __callback() function which created an unnecessary level of
>> indirection to execute the tranfer callback .xfer_cb
>>
>> Unfortunately the commit also changed the semantics slightly since that
>> function used to check if the request was not NULL before attempting to
>> execute the callback function. Not checking this could lead to a kernel
>> NULL pointer dereference error.
> 
> This should not happen, but I guess it can happen when terminal_all() is 

I should had mentioned before that this patch is not trying to fix a theoretical
issue but a kernel oops when booting linux next-20140718 on a Exynos5420 SoC
based Chromebook 2 machine.

I'm sending as an attachment the complete kernel crash log but the problem
happens when the spi_master .unprepare_transfer_hardware function handler in the
spi-s3c64xx driver tries to release a DMA channel:

s3c64xx_spi_unprepare_transfer() ->
dma_release_channel() ->
dma_chan_put() ->
chan->device->device_free_chan_resources() ->
pl330_free_chan_resources() ->
pl330_release_channel() ->
dma_pl330_rqcb()

> called. (It's wrong to try to complete a descriptor from terminal_all() in 
> the first place, but that's a different issue)

If this should not really happen and this patch is only a workaround since the
bug is elsewhere, please give me some hints and I'll try to fix it properly. I'm
not familiar with the PL330 DMA controller but just found what was the NULL
pointer being dereferenced and looked at your changes to see what was different now.

> 
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@...labora.co.uk>
> 
> Acked-by: Lars-Peter Clausen <lars@...afoo.de>
> 

Thanks a lot and best regards,
Javier

View attachment "kernel_oops.log" of type "text/x-log" (3454 bytes)