linux-kernel - Re: [BUG] ata: pata_marvell: Warning when probing the module

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]

Message-ID: <e7bd823c-c387-00e7-3060-97f224df6da4@opensource.wdc.com>
Date:   Wed, 20 Apr 2022 12:04:14 +0900
From:   Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To:     Zheyu Ma <zheyuma97@...il.com>
Cc:     Sergey Shtylyov <s.shtylyov@....ru>, linux-ide@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] ata: pata_marvell: Warning when probing the module

On 4/20/22 11:21, Zheyu Ma wrote:
> On Wed, Apr 13, 2022 at 11:42 AM Damien Le Moal
> <damien.lemoal@...nsource.wdc.com> wrote:
>>
>> On 4/12/22 15:34, Zheyu Ma wrote:
>>> On Mon, Apr 11, 2022 at 7:53 AM Damien Le Moal
>>> <damien.lemoal@...nsource.wdc.com> wrote:
>>>>
>>>> On 4/10/22 15:30, Zheyu Ma wrote:
>>>>> Hello,
>>>>>
>>>>> I found a bug in the pata_marvell module.
>>>>> When probing the driver, it seems to trigger the error path and
>>>>> executes the function marvell_cable_detect(), but the
>>>>> 'ap->ioaddr.bmdma_addr' is not initialized, which causes a warning.
>>>>
>>>> I do not have this hardware so I cannot debug this. Please debug it and
>>>> send a patch. bmdma_addr is normally set in ata_pci_bmdma_init(), but some
>>>> drivers set it manually in their probe functions. No idea about the
>>>> marvell driver, I have not checked it.
>>>
>>> To be honest I don't have a good solution to this problem, because
>>> other drivers don't have similar behavior. The marvell driver doesn't
>>> even initialize 'bmdma_addr' before calling 'cable_detect'.
>>
>> Then this is the bug that needs to be fixed, no ?
>>
>>> So a simple idea I have is to check if 'bmdma_addr' is 0 before
>>> reading it and if so return the error code ATA_CBL_NONE.
>>
>> And if indeed, even after it is initialized it is still 0, then yes, this
>> change seems sensible.
> 
> Sorry for the late reply, I found the root cause of this issue.
> The marvell driver execute the ata_pci_bmdma_init() function, but the
> driver just returned at the following code snippet.
> 
> if (pci_resource_start(pdev, 4) == 0) {
>       ata_bmdma_nodma(host, "BAR4 is zero");
>       return;
> }
> 
> So the driver didn't initialize the 'bmdma_addr' but used it in the
> cable_detect() function.
> It seems that the problem is caused by the hardware, is this a bug
> that we should fix?

So it looks like your adapter is saying: I do not support DMA.
In that case, having bmdma_addr as 0 should be expected and
pata_marvel_cable_detect() should check the address before attempting an
ioread8(). It is weird that the cable information is in that bar though...

In any case, you should check the adapter specs to verify how the cable
type can be detected. And if unknown when bmdma_addr is 0, then just
return ATA_CBL_PATA_UNK.

> 
> Zheyu Ma
> 
> 
> Zheyu Ma


-- 
Damien Le Moal
Western Digital Research