Baokun Li (2):
sata_fsl: fix UAF in sata_fsl_port_stop when rmmod sata_fsl
sata_fsl: fix warning in remove_proc_entry when rmmod sata_fsl
drivers/ata/sata_fsl.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
--
2.31.1
Trying to remove the fsl-sata module in the PPC64 GNU/Linux
leads to the following warning:
------------[ cut here ]------------
remove_proc_entry: removing non-empty directory 'irq/69',
leaking at least 'fsl-sata[ff0221000.sata]'
WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
.remove_proc_entry+0x20c/0x220
IRQMASK: 0
NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
LR [c000000000338268] .remove_proc_entry+0x208/0x220
Call Trace:
.remove_proc_entry+0x208/0x220 (unreliable)
.unregister_irq_proc+0x104/0x140
.free_desc+0x44/0xb0
.irq_free_descs+0x9c/0xf0
.irq_dispose_mapping+0x64/0xa0
.sata_fsl_remove+0x58/0xa0 [sata_fsl]
.platform_drv_remove+0x40/0x90
.device_release_driver_internal+0x160/0x2c0
.driver_detach+0x64/0xd0
.bus_remove_driver+0x70/0xf0
.driver_unregister+0x38/0x80
.platform_driver_unregister+0x14/0x30
.fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
---[ end trace 0ea876d4076908f5 ]---
The driver creates the mapping by calling irq_of_parse_and_map(),
so it also has to dispose the mapping. But the easy way out is to
simply use platform_get_irq() instead of irq_of_parse_map().
In this case the mapping is not managed by the device but by
the of core, so the device has not to dispose the mapping.
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Baokun Li <[email protected]>
---
drivers/ata/sata_fsl.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 30759fd1c3a2..011daac4a14e 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
host_priv->ssr_base = ssr_base;
host_priv->csr_base = csr_base;
- irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
+ irq = platform_get_irq(ofdev, 0);
if (!irq) {
dev_err(&ofdev->dev, "invalid irq from platform\n");
goto error_exit_with_cleanup;
@@ -1570,8 +1570,6 @@ static int sata_fsl_remove(struct platform_device *ofdev)
ata_host_detach(host);
- irq_dispose_mapping(host_priv->irq);
-
return 0;
}
--
2.31.1
When the `rmmod sata_fsl.ko` command is executed in the PPC64 GNU/Linux,
a bug is reported:
==================================================================
BUG: Unable to handle kernel data access on read at 0x80000800805b502c
Oops: Kernel access of bad area, sig: 11 [#1]
NIP [c0000000000388a4] .ioread32+0x4/0x20
LR [80000000000c6034] .sata_fsl_port_stop+0x44/0xe0 [sata_fsl]
Call Trace:
.free_irq+0x1c/0x4e0 (unreliable)
.ata_host_stop+0x74/0xd0 [libata]
.release_nodes+0x330/0x3f0
.device_release_driver_internal+0x178/0x2c0
.driver_detach+0x64/0xd0
.bus_remove_driver+0x70/0xf0
.driver_unregister+0x38/0x80
.platform_driver_unregister+0x14/0x30
.fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
.__se_sys_delete_module+0x1ec/0x2d0
.system_call_exception+0xfc/0x1f0
system_call_common+0xf8/0x200
==================================================================
The triggering of the BUG is shown in the following stack:
driver_detach
device_release_driver_internal
__device_release_driver
drv->remove(dev) --> platform_drv_remove/platform_remove
drv->remove(dev) --> sata_fsl_remove
iounmap(host_priv->hcr_base); <---- unmap
kfree(host_priv); <---- free
devres_release_all
release_nodes
dr->node.release(dev, dr->data) --> ata_host_stop
ap->ops->port_stop(ap) --> sata_fsl_port_stop
ioread32(hcr_base + HCONTROL) <---- UAF
host->ops->host_stop(host)
The iounmap(host_priv->hcr_base) and kfree(host_priv) commands should
not be executed in drv->remove. These commands should be executed in
host_stop after port_stop. Therefore, we move these commands to the
new function sata_fsl_host_stop and bind the new function to host_stop
by referring to achi.
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Baokun Li <[email protected]>
---
drivers/ata/sata_fsl.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index e5838b23c9e0..30759fd1c3a2 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -1430,12 +1430,25 @@ static struct ata_port_operations sata_fsl_ops = {
.pmp_detach = sata_fsl_pmp_detach,
};
+static void sata_fsl_host_stop(struct ata_host *host)
+{
+ struct sata_fsl_host_priv *host_priv = host->private_data;
+
+ iounmap(host_priv->hcr_base);
+ kfree(host_priv);
+}
+
+static struct ata_port_operations sata_fsl_platform_ops = {
+ .inherits = &sata_fsl_ops,
+ .host_stop = sata_fsl_host_stop,
+};
+
static const struct ata_port_info sata_fsl_port_info[] = {
{
.flags = SATA_FSL_HOST_FLAGS,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
- .port_ops = &sata_fsl_ops,
+ .port_ops = &sata_fsl_platform_ops,
},
};
@@ -1558,8 +1571,6 @@ static int sata_fsl_remove(struct platform_device *ofdev)
ata_host_detach(host);
irq_dispose_mapping(host_priv->irq);
- iounmap(host_priv->hcr_base);
- kfree(host_priv);
return 0;
}
--
2.31.1
Hello!
On 19.11.2021 7:11, Baokun Li wrote:
> Trying to remove the fsl-sata module in the PPC64 GNU/Linux
> leads to the following warning:
> ------------[ cut here ]------------
> remove_proc_entry: removing non-empty directory 'irq/69',
> leaking at least 'fsl-sata[ff0221000.sata]'
> WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
> .remove_proc_entry+0x20c/0x220
> IRQMASK: 0
> NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
> LR [c000000000338268] .remove_proc_entry+0x208/0x220
> Call Trace:
> .remove_proc_entry+0x208/0x220 (unreliable)
> .unregister_irq_proc+0x104/0x140
> .free_desc+0x44/0xb0
> .irq_free_descs+0x9c/0xf0
> .irq_dispose_mapping+0x64/0xa0
> .sata_fsl_remove+0x58/0xa0 [sata_fsl]
> .platform_drv_remove+0x40/0x90
> .device_release_driver_internal+0x160/0x2c0
> .driver_detach+0x64/0xd0
> .bus_remove_driver+0x70/0xf0
> .driver_unregister+0x38/0x80
> .platform_driver_unregister+0x14/0x30
> .fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
> ---[ end trace 0ea876d4076908f5 ]---
>
> The driver creates the mapping by calling irq_of_parse_and_map(),
> so it also has to dispose the mapping. But the easy way out is to
> simply use platform_get_irq() instead of irq_of_parse_map().
Not that easy. :-)
> In this case the mapping is not managed by the device but by
> the of core, so the device has not to dispose the mapping.
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Baokun Li <[email protected]>
> ---
> drivers/ata/sata_fsl.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index 30759fd1c3a2..011daac4a14e 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
> host_priv->ssr_base = ssr_base;
> host_priv->csr_base = csr_base;
>
> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> + irq = platform_get_irq(ofdev, 0);
> if (!irq) {
if (irq < 0) {
platform_get_irq() returns negative error codes, not 0 on failure.
[...]
MBR, Sergey
On 11/19/21 13:11, Baokun Li wrote:
> Trying to remove the fsl-sata module in the PPC64 GNU/Linux
> leads to the following warning:
> ------------[ cut here ]------------
> remove_proc_entry: removing non-empty directory 'irq/69',
> leaking at least 'fsl-sata[ff0221000.sata]'
> WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
> .remove_proc_entry+0x20c/0x220
> IRQMASK: 0
> NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
> LR [c000000000338268] .remove_proc_entry+0x208/0x220
> Call Trace:
> .remove_proc_entry+0x208/0x220 (unreliable)
> .unregister_irq_proc+0x104/0x140
> .free_desc+0x44/0xb0
> .irq_free_descs+0x9c/0xf0
> .irq_dispose_mapping+0x64/0xa0
> .sata_fsl_remove+0x58/0xa0 [sata_fsl]
> .platform_drv_remove+0x40/0x90
> .device_release_driver_internal+0x160/0x2c0
> .driver_detach+0x64/0xd0
> .bus_remove_driver+0x70/0xf0
> .driver_unregister+0x38/0x80
> .platform_driver_unregister+0x14/0x30
> .fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
> ---[ end trace 0ea876d4076908f5 ]---
>
> The driver creates the mapping by calling irq_of_parse_and_map(),
> so it also has to dispose the mapping. But the easy way out is to
> simply use platform_get_irq() instead of irq_of_parse_map().
>
> In this case the mapping is not managed by the device but by
> the of core, so the device has not to dispose the mapping.
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Baokun Li <[email protected]>
> ---
> drivers/ata/sata_fsl.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index 30759fd1c3a2..011daac4a14e 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
> host_priv->ssr_base = ssr_base;
> host_priv->csr_base = csr_base;
>
> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> + irq = platform_get_irq(ofdev, 0);
> if (!irq) {
Please see the kdoc comment for platform_get_irq() in
drivers/base/platform.c. The error check must be "if (irq < 0)".
Can you send a V2 with that fixed and tested ?
> dev_err(&ofdev->dev, "invalid irq from platform\n");
> goto error_exit_with_cleanup;
> @@ -1570,8 +1570,6 @@ static int sata_fsl_remove(struct platform_device *ofdev)
>
> ata_host_detach(host);
>
> - irq_dispose_mapping(host_priv->irq);
> -
> return 0;
> }
>
>
--
Damien Le Moal
Western Digital Research
在 2021/11/19 23:43, Sergei Shtylyov 写道:
> Hello!
>
> On 19.11.2021 7:11, Baokun Li wrote:
>
>> Trying to remove the fsl-sata module in the PPC64 GNU/Linux
>> leads to the following warning:
>> ------------[ cut here ]------------
>> remove_proc_entry: removing non-empty directory 'irq/69',
>> leaking at least 'fsl-sata[ff0221000.sata]'
>> WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
>> .remove_proc_entry+0x20c/0x220
>> IRQMASK: 0
>> NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
>> LR [c000000000338268] .remove_proc_entry+0x208/0x220
>> Call Trace:
>> .remove_proc_entry+0x208/0x220 (unreliable)
>> .unregister_irq_proc+0x104/0x140
>> .free_desc+0x44/0xb0
>> .irq_free_descs+0x9c/0xf0
>> .irq_dispose_mapping+0x64/0xa0
>> .sata_fsl_remove+0x58/0xa0 [sata_fsl]
>> .platform_drv_remove+0x40/0x90
>> .device_release_driver_internal+0x160/0x2c0
>> .driver_detach+0x64/0xd0
>> .bus_remove_driver+0x70/0xf0
>> .driver_unregister+0x38/0x80
>> .platform_driver_unregister+0x14/0x30
>> .fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
>> ---[ end trace 0ea876d4076908f5 ]---
>>
>> The driver creates the mapping by calling irq_of_parse_and_map(),
>> so it also has to dispose the mapping. But the easy way out is to
>> simply use platform_get_irq() instead of irq_of_parse_map().
>
> Not that easy. :-)
>
>> In this case the mapping is not managed by the device but by
>> the of core, so the device has not to dispose the mapping.
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Signed-off-by: Baokun Li <[email protected]>
>> ---
>> drivers/ata/sata_fsl.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>> index 30759fd1c3a2..011daac4a14e 100644
>> --- a/drivers/ata/sata_fsl.c
>> +++ b/drivers/ata/sata_fsl.c
>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct
>> platform_device *ofdev)
>> host_priv->ssr_base = ssr_base;
>> host_priv->csr_base = csr_base;
>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>> + irq = platform_get_irq(ofdev, 0);
>> if (!irq) {
>
> if (irq < 0) {
>
> platform_get_irq() returns negative error codes, not 0 on failure.
>
> [...]
>
> MBR, Sergey
> .
I didn't notice the change in this return value, and the test didn't
cover the error branch.
Thank you very much for your correction.
I'm about to send a patch v2 with the changes suggested by you.
--
With Best Regards,
Baokun Li
.
在 2021/11/19 23:43, Sergei Shtylyov 写道:
> Hello!
>
> On 19.11.2021 7:11, Baokun Li wrote:
>
>> Trying to remove the fsl-sata module in the PPC64 GNU/Linux
>> leads to the following warning:
>> ------------[ cut here ]------------
>> remove_proc_entry: removing non-empty directory 'irq/69',
>> leaking at least 'fsl-sata[ff0221000.sata]'
>> WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
>> .remove_proc_entry+0x20c/0x220
>> IRQMASK: 0
>> NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
>> LR [c000000000338268] .remove_proc_entry+0x208/0x220
>> Call Trace:
>> .remove_proc_entry+0x208/0x220 (unreliable)
>> .unregister_irq_proc+0x104/0x140
>> .free_desc+0x44/0xb0
>> .irq_free_descs+0x9c/0xf0
>> .irq_dispose_mapping+0x64/0xa0
>> .sata_fsl_remove+0x58/0xa0 [sata_fsl]
>> .platform_drv_remove+0x40/0x90
>> .device_release_driver_internal+0x160/0x2c0
>> .driver_detach+0x64/0xd0
>> .bus_remove_driver+0x70/0xf0
>> .driver_unregister+0x38/0x80
>> .platform_driver_unregister+0x14/0x30
>> .fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
>> ---[ end trace 0ea876d4076908f5 ]---
>>
>> The driver creates the mapping by calling irq_of_parse_and_map(),
>> so it also has to dispose the mapping. But the easy way out is to
>> simply use platform_get_irq() instead of irq_of_parse_map().
>
> Not that easy. :-)
>
>> In this case the mapping is not managed by the device but by
>> the of core, so the device has not to dispose the mapping.
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Signed-off-by: Baokun Li <[email protected]>
>> ---
>> drivers/ata/sata_fsl.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>> index 30759fd1c3a2..011daac4a14e 100644
>> --- a/drivers/ata/sata_fsl.c
>> +++ b/drivers/ata/sata_fsl.c
>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct
>> platform_device *ofdev)
>> host_priv->ssr_base = ssr_base;
>> host_priv->csr_base = csr_base;
>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>> + irq = platform_get_irq(ofdev, 0);
>> if (!irq) {
>
> if (irq < 0) {
>
> platform_get_irq() returns negative error codes, not 0 on failure.
>
> [...]
>
> MBR, Sergey
> .
I didn't notice the change in this return value, and the test didn't
cover the error branch.
Thank you very much for your advice.
I'm about to send a patch v2 with the changes suggested by you.
--
With Best Regards,
Baokun Li
.
在 2021/11/20 6:18, Damien Le Moal 写道:
> On 11/19/21 13:11, Baokun Li wrote:
>> Trying to remove the fsl-sata module in the PPC64 GNU/Linux
>> leads to the following warning:
>> ------------[ cut here ]------------
>> remove_proc_entry: removing non-empty directory 'irq/69',
>> leaking at least 'fsl-sata[ff0221000.sata]'
>> WARNING: CPU: 3 PID: 1048 at fs/proc/generic.c:722
>> .remove_proc_entry+0x20c/0x220
>> IRQMASK: 0
>> NIP [c00000000033826c] .remove_proc_entry+0x20c/0x220
>> LR [c000000000338268] .remove_proc_entry+0x208/0x220
>> Call Trace:
>> .remove_proc_entry+0x208/0x220 (unreliable)
>> .unregister_irq_proc+0x104/0x140
>> .free_desc+0x44/0xb0
>> .irq_free_descs+0x9c/0xf0
>> .irq_dispose_mapping+0x64/0xa0
>> .sata_fsl_remove+0x58/0xa0 [sata_fsl]
>> .platform_drv_remove+0x40/0x90
>> .device_release_driver_internal+0x160/0x2c0
>> .driver_detach+0x64/0xd0
>> .bus_remove_driver+0x70/0xf0
>> .driver_unregister+0x38/0x80
>> .platform_driver_unregister+0x14/0x30
>> .fsl_sata_driver_exit+0x18/0xa20 [sata_fsl]
>> ---[ end trace 0ea876d4076908f5 ]---
>>
>> The driver creates the mapping by calling irq_of_parse_and_map(),
>> so it also has to dispose the mapping. But the easy way out is to
>> simply use platform_get_irq() instead of irq_of_parse_map().
>>
>> In this case the mapping is not managed by the device but by
>> the of core, so the device has not to dispose the mapping.
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Signed-off-by: Baokun Li <[email protected]>
>> ---
>> drivers/ata/sata_fsl.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>> index 30759fd1c3a2..011daac4a14e 100644
>> --- a/drivers/ata/sata_fsl.c
>> +++ b/drivers/ata/sata_fsl.c
>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
>> host_priv->ssr_base = ssr_base;
>> host_priv->csr_base = csr_base;
>>
>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>> + irq = platform_get_irq(ofdev, 0);
>> if (!irq) {
> Please see the kdoc comment for platform_get_irq() in
> drivers/base/platform.c. The error check must be "if (irq < 0)".
>
> Can you send a V2 with that fixed and tested ?
>
>> dev_err(&ofdev->dev, "invalid irq from platform\n");
>> goto error_exit_with_cleanup;
>> @@ -1570,8 +1570,6 @@ static int sata_fsl_remove(struct platform_device *ofdev)
>>
>> ata_host_detach(host);
>>
>> - irq_dispose_mapping(host_priv->irq);
>> -
>> return 0;
>> }
>>
>>
>
I didn't notice the change in this return value, and the test didn't
cover the error branch.
Thank you very much for your advice.
I'm about to send a patch v2 with the changes suggested by you.
-- With Best Regards, Baokun Li .
On 11/20/21 00:43, Sergei Shtylyov wrote:
>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>> index 30759fd1c3a2..011daac4a14e 100644
>> --- a/drivers/ata/sata_fsl.c
>> +++ b/drivers/ata/sata_fsl.c
>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
>> host_priv->ssr_base = ssr_base;
>> host_priv->csr_base = csr_base;
>>
>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>> + irq = platform_get_irq(ofdev, 0);
>> if (!irq) {
>
> if (irq < 0) {
>
> platform_get_irq() returns negative error codes, not 0 on failure.
Sergei,
By the way, the kdoc comment for platform_get_irq() says:
"Return: non-zero IRQ number on success, negative error number on failure."
But irq 0 is valid, isn't it ? So shouldn't this be changed to something
like:
"Return: IRQ number on success, negative error number on failure."
--
Damien Le Moal
Western Digital Research
On 20.11.2021 9:08, Damien Le Moal wrote:
> On 11/20/21 00:43, Sergei Shtylyov wrote:
>>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>>> index 30759fd1c3a2..011daac4a14e 100644
>>> --- a/drivers/ata/sata_fsl.c
>>> +++ b/drivers/ata/sata_fsl.c
>>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
>>> host_priv->ssr_base = ssr_base;
>>> host_priv->csr_base = csr_base;
>>>
>>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>>> + irq = platform_get_irq(ofdev, 0);
>>> if (!irq) {
>>
>> if (irq < 0) {
>>
>> platform_get_irq() returns negative error codes, not 0 on failure.
>
> Sergei,
>
> By the way, the kdoc comment for platform_get_irq() says:
>
> "Return: non-zero IRQ number on success, negative error number on failure."
>
> But irq 0 is valid, isn't it ? So shouldn't this be changed to something
> like:
>
> "Return: IRQ number on success, negative error number on failure."
No, it's not valid (the current code WARN()s about it) and won't be
returned anymore after my patch [1] gets applied.
[1] https://marc.info/?l=linux-kernel&m=163623041902285
MBR, Sergei
On 2021/11/20 18:51, Sergei Shtylyov wrote:
> On 20.11.2021 9:08, Damien Le Moal wrote:
>> On 11/20/21 00:43, Sergei Shtylyov wrote:
>>>> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
>>>> index 30759fd1c3a2..011daac4a14e 100644
>>>> --- a/drivers/ata/sata_fsl.c
>>>> +++ b/drivers/ata/sata_fsl.c
>>>> @@ -1493,7 +1493,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
>>>> host_priv->ssr_base = ssr_base;
>>>> host_priv->csr_base = csr_base;
>>>>
>>>> - irq = irq_of_parse_and_map(ofdev->dev.of_node, 0);
>>>> + irq = platform_get_irq(ofdev, 0);
>>>> if (!irq) {
>>>
>>> if (irq < 0) {
>>>
>>> platform_get_irq() returns negative error codes, not 0 on failure.
>>
>> Sergei,
>>
>> By the way, the kdoc comment for platform_get_irq() says:
>>
>> "Return: non-zero IRQ number on success, negative error number on failure."
>>
>> But irq 0 is valid, isn't it ? So shouldn't this be changed to something
>> like:
>>
>> "Return: IRQ number on success, negative error number on failure."
>
> No, it's not valid (the current code WARN()s about it) and won't be
> returned anymore after my patch [1] gets applied.
>
> [1] https://marc.info/?l=linux-kernel&m=163623041902285
OK. Got it. Thanks.
>
> MBR, Sergei
>
--
Damien Le Moal
Western Digital Research