diff mbox

PCI: stop crashing in pci_release_resource v2

Message ID 20180221090715.2853-1-christian.koenig@amd.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Christian König Feb. 21, 2018, 9:07 a.m. UTC
Is it entirely possible that the BIOS wasn't able to assign resources to
a device. In this case don't crash in pci_release_resource() when we try
to resize the resource.

v2: keep printing the info that we try to release the BAR

Signed-off-by: Christian König <christian.koenig@amd.com>
CC: stable@vger.kernel.org
---
 drivers/pci/setup-res.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Andy Shevchenko Feb. 21, 2018, 7:24 p.m. UTC | #1
On Wed, Feb 21, 2018 at 11:07 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Is it entirely possible that the BIOS wasn't able to assign resources to
> a device. In this case don't crash in pci_release_resource() when we try
> to resize the resource.
>
> v2: keep printing the info that we try to release the BAR
>

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Christian König <christian.koenig@amd.com>
> CC: stable@vger.kernel.org
> ---
>  drivers/pci/setup-res.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e815111f3f81..c0f091614805 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -402,6 +402,10 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>         struct resource *res = dev->resource + resno;
>
>         dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
> +
> +       if (!res->parent)
> +               return;
> +
>         release_resource(res);
>         res->end = resource_size(res) - 1;
>         res->start = 0;
> --
> 2.14.1
>
Bjorn Helgaas Feb. 26, 2018, 8:56 p.m. UTC | #2
On Wed, Feb 21, 2018 at 10:07:15AM +0100, Christian König wrote:
> Is it entirely possible that the BIOS wasn't able to assign resources to
> a device. In this case don't crash in pci_release_resource() when we try
> to resize the resource.
> 
> v2: keep printing the info that we try to release the BAR
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> CC: stable@vger.kernel.org

I think we probably want this in v4.16, since it seems to be a fix for
8bb705e3e79d ("PCI: Add pci_resize_resource() for resizing BARs"),
which appeared in v4.15.

What does the crash look like?  I might include a few lines in the
changelog to help people find the fix.

I added a Fixes: tag and Andy's Reviewed-by already.

> ---
>  drivers/pci/setup-res.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e815111f3f81..c0f091614805 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -402,6 +402,10 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>  	struct resource *res = dev->resource + resno;
>  
>  	dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
> +
> +	if (!res->parent)
> +		return;
> +
>  	release_resource(res);
>  	res->end = resource_size(res) - 1;
>  	res->start = 0;
> -- 
> 2.14.1
>
Bjorn Helgaas March 1, 2018, 6:49 p.m. UTC | #3
On Wed, Feb 21, 2018 at 10:07:15AM +0100, Christian König wrote:
> Is it entirely possible that the BIOS wasn't able to assign resources to
> a device. In this case don't crash in pci_release_resource() when we try
> to resize the resource.
> 
> v2: keep printing the info that we try to release the BAR
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> CC: stable@vger.kernel.org

I was hoping for a dmesg snippet, but I added a Fixes: tag and applied
this to for-linus for v4.16.

> ---
>  drivers/pci/setup-res.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e815111f3f81..c0f091614805 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -402,6 +402,10 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>  	struct resource *res = dev->resource + resno;
>  
>  	dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
> +
> +	if (!res->parent)
> +		return;
> +
>  	release_resource(res);
>  	res->end = resource_size(res) - 1;
>  	res->start = 0;
> -- 
> 2.14.1
>
Christian König March 2, 2018, 7:55 a.m. UTC | #4
Am 01.03.2018 um 19:49 schrieb Bjorn Helgaas:
> On Wed, Feb 21, 2018 at 10:07:15AM +0100, Christian König wrote:
>> Is it entirely possible that the BIOS wasn't able to assign resources to
>> a device. In this case don't crash in pci_release_resource() when we try
>> to resize the resource.
>>
>> v2: keep printing the info that we try to release the BAR
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> CC: stable@vger.kernel.org
> I was hoping for a dmesg snippet, but I added a Fixes: tag and applied
> this to for-linus for v4.16.

Sorry, been on vacation and missed your mail.

I don't have a dmesg of hand either, cause the reporter provided only a 
mail with a picture of the crash :)

But he confirmed that the patch worked.

Thanks,
Christian.

>
>> ---
>>   drivers/pci/setup-res.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index e815111f3f81..c0f091614805 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -402,6 +402,10 @@ void pci_release_resource(struct pci_dev *dev, int resno)
>>   	struct resource *res = dev->resource + resno;
>>   
>>   	dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
>> +
>> +	if (!res->parent)
>> +		return;
>> +
>>   	release_resource(res);
>>   	res->end = resource_size(res) - 1;
>>   	res->start = 0;
>> -- 
>> 2.14.1
>>
diff mbox

Patch

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index e815111f3f81..c0f091614805 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -402,6 +402,10 @@  void pci_release_resource(struct pci_dev *dev, int resno)
 	struct resource *res = dev->resource + resno;
 
 	dev_info(&dev->dev, "BAR %d: releasing %pR\n", resno, res);
+
+	if (!res->parent)
+		return;
+
 	release_resource(res);
 	res->end = resource_size(res) - 1;
 	res->start = 0;