linux-kernel - Re: [PATCH] ata: dwc_460ex: Avoid potential NULL pointer dereference

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] [day] [month] [year] [list]

Date:   Sun, 24 Mar 2019 11:10:02 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Aditya Pakki <pakki001@....edu>
Cc:     kjlu@....edu, linux-ide@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ata: dwc_460ex: Avoid potential NULL pointer dereference

On 3/24/19 10:47 AM, Aditya Pakki wrote:
> On 3/24/19 11:28 AM, Jens Axboe wrote:
>> On 3/4/19 4:08 PM, Aditya Pakki wrote:
>>> dma_async_tx_descriptor can contain a NULL variable and using
>>> it in dmaengine_submit without checking can crash the process.
>>> This patch avoids such a scenario.
>>>
>>> Signed-off-by: Aditya Pakki <pakki001@....edu>
>>> ---
>>>  drivers/ata/sata_dwc_460ex.c | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
>>> index 6f142aa54f5f..44a0d7a1ef54 100644
>>> --- a/drivers/ata/sata_dwc_460ex.c
>>> +++ b/drivers/ata/sata_dwc_460ex.c
>>> @@ -1052,8 +1052,10 @@ static void sata_dwc_bmdma_start_by_tag(struct ata_queued_cmd *qc, u8 tag)
>>>  					SATA_DWC_DMACR_RXCHEN);
>>>  
>>>  		/* Enable AHB DMA transfer on the specified channel */
>>> -		dmaengine_submit(desc);
>>> -		dma_async_issue_pending(hsdevp->chan);
>>> +		if (desc) {
>>> +			dmaengine_submit(desc);
>>> +			dma_async_issue_pending(hsdevp->chan);
>>> +		}
>>>  	}
>>>  }
>>
>> Hmm, if desc == NULL, is that an error condition?
>>
> Jens,
> In dmaengine_submit, the desc variable is dereferenced without a check for NULL.

Yes, that's not my point. My point is if desc is NULL, and you just work around it
by not issuing the DMA, are you just going to hang the drive/box? Should you be
erroring the IO at that point instead?

-- 
Jens Axboe