1. Patchew
  2. linux
  3. [PATCH drm-dp 00/10] Fix hibmc driver bugs
  4. View patch

[PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local

Yongbang Shi posted 10 patches 6 months, 3 weeks ago
There is a newer version of this series
[PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Yongbang Shi 6 months, 3 weeks ago 
From: Baihan Li <libaihan@huawei.com>

The local variable of irq name is passed to devm_request_threaded_irq(),
which will make request_irq failed. Using the global irq name instead
of it to fix.

Fixes: b11bc1ae4658 ("drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD")
Signed-off-by: Baihan Li <libaihan@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 768b97f9e74a..4cdcc34070ee 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -32,7 +32,7 @@
 
 DEFINE_DRM_GEM_FOPS(hibmc_fops);
 
-static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "vblank", "hpd" };
+static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "hibmc-vblank", "hibmc-hpd" };
 
 static irqreturn_t hibmc_interrupt(int irq, void *arg)
 {
@@ -277,7 +277,6 @@ static void hibmc_unload(struct drm_device *dev)
 static int hibmc_msi_init(struct drm_device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	char name[32] = {0};
 	int valid_irq_num;
 	int irq;
 	int ret;
@@ -292,9 +291,6 @@ static int hibmc_msi_init(struct drm_device *dev)
 	valid_irq_num = ret;
 
 	for (int i = 0; i < valid_irq_num; i++) {
-		snprintf(name, ARRAY_SIZE(name) - 1, "%s-%s-%s",
-			 dev->driver->name, pci_name(pdev), g_irqs_names_map[i]);
-
 		irq = pci_irq_vector(pdev, i);
 
 		if (i)
@@ -302,10 +298,10 @@ static int hibmc_msi_init(struct drm_device *dev)
 			ret = devm_request_threaded_irq(&pdev->dev, irq,
 							hibmc_dp_interrupt,
 							hibmc_dp_hpd_isr,
-							IRQF_SHARED, name, dev);
+							IRQF_SHARED, g_irqs_names_map[i], dev);
 		else
 			ret = devm_request_irq(&pdev->dev, irq, hibmc_interrupt,
-					       IRQF_SHARED, name, dev);
+					       IRQF_SHARED, g_irqs_names_map[i], dev);
 		if (ret) {
 			drm_err(dev, "install irq failed: %d\n", ret);
 			return ret;
-- 
2.33.0
Re: [PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Dmitry Baryshkov 6 months, 1 week ago 
On Fri, May 30, 2025 at 05:54:25PM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@huawei.com>
> 
> The local variable of irq name is passed to devm_request_threaded_irq(),
> which will make request_irq failed. Using the global irq name instead
> of it to fix.

This doesn't explain, why does it fail and which IRQ name is actually
expected.

> 
> Fixes: b11bc1ae4658 ("drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD")
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 768b97f9e74a..4cdcc34070ee 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -32,7 +32,7 @@
>  
>  DEFINE_DRM_GEM_FOPS(hibmc_fops);
>  
> -static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "vblank", "hpd" };
> +static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "hibmc-vblank", "hibmc-hpd" };

Please point to the corresponding IRQ names as currently implemented in
the upstream kernel.

>  
>  static irqreturn_t hibmc_interrupt(int irq, void *arg)
>  {

-- 
With best wishes
Dmitry
Re: [PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Yongbang Shi 6 months, 1 week ago 
> On Fri, May 30, 2025 at 05:54:25PM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@huawei.com>
>>
>> The local variable of irq name is passed to devm_request_threaded_irq(),
>> which will make request_irq failed. Using the global irq name instead
>> of it to fix.
> This doesn't explain, why does it fail and which IRQ name is actually
> expected.

The local variable is passed in request_irq (), and there will be use after free problem.


>> Fixes: b11bc1ae4658 ("drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD")
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 +++-------
>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index 768b97f9e74a..4cdcc34070ee 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -32,7 +32,7 @@
>>   
>>   DEFINE_DRM_GEM_FOPS(hibmc_fops);
>>   
>> -static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "vblank", "hpd" };
>> +static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "hibmc-vblank", "hibmc-hpd" };
> Please point to the corresponding IRQ names as currently implemented in
> the upstream kernel.

Ok.


>>   
>>   static irqreturn_t hibmc_interrupt(int irq, void *arg)
>>   {
Re: [PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Dmitry Baryshkov 6 months, 1 week ago 
On 09/06/2025 17:50, Yongbang Shi wrote:
> 
>> On Fri, May 30, 2025 at 05:54:25PM +0800, Yongbang Shi wrote:
>>> From: Baihan Li <libaihan@huawei.com>
>>>
>>> The local variable of irq name is passed to devm_request_threaded_irq(),
>>> which will make request_irq failed. Using the global irq name instead
>>> of it to fix.
>> This doesn't explain, why does it fail and which IRQ name is actually
>> expected.
> 
> The local variable is passed in request_irq (), and there will be use 
> after free problem.

This needs to be explained (in details) in the commit message.

> 
> 
>>> Fixes: b11bc1ae4658 ("drm/hisilicon/hibmc: Add MSI irq getting and 
>>> requesting for HPD")
>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>> ---
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 +++-------
>>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/ 
>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> index 768b97f9e74a..4cdcc34070ee 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -32,7 +32,7 @@
>>>   DEFINE_DRM_GEM_FOPS(hibmc_fops);
>>> -static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "vblank", 
>>> "hpd" };
>>> +static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "hibmc- 
>>> vblank", "hibmc-hpd" };
>> Please point to the corresponding IRQ names as currently implemented in
>> the upstream kernel.
> 
> Ok.

I was thinking in terms of IRQ lookup. You can ignore this comment (it 
makes me wonder, how did you understand it, if you responded with Ok).

> 
> 
>>>   static irqreturn_t hibmc_interrupt(int irq, void *arg)
>>>   {


-- 
With best wishes
Dmitry
Re: [PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Zenghui Yu 6 months ago 
On 2025/6/9 22:53, Dmitry Baryshkov wrote:
> On 09/06/2025 17:50, Yongbang Shi wrote:
> >
> > > On Fri, May 30, 2025 at 05:54:25PM +0800, Yongbang Shi wrote:
> > > > From: Baihan Li <libaihan@huawei.com>
> > > >
> > > > The local variable of irq name is passed to
> > > > devm_request_threaded_irq(),
> > > > which will make request_irq failed. Using the global irq name instead
> > > > of it to fix.
> > > This doesn't explain, why does it fail and which IRQ name is actually
> > > expected.
> >
> > The local variable is passed in request_irq (), and there will be use
> > after free problem.
> 
> This needs to be explained (in details) in the commit message.

+1. And I hope this can be fixed as soon as possible. I've run into
several OOPS with that.

Thanks,
Zenghui
Re: [PATCH drm-dp 03/10] drm/hisilicon/hibmc: fix irq_request()'s irq name variable is local
Posted by Yongbang Shi 6 months, 1 week ago 
> On 09/06/2025 17:50, Yongbang Shi wrote:
>>
>>> On Fri, May 30, 2025 at 05:54:25PM +0800, Yongbang Shi wrote:
>>>> From: Baihan Li <libaihan@huawei.com>
>>>>
>>>> The local variable of irq name is passed to 
>>>> devm_request_threaded_irq(),
>>>> which will make request_irq failed. Using the global irq name instead
>>>> of it to fix.
>>> This doesn't explain, why does it fail and which IRQ name is actually
>>> expected.
>>
>> The local variable is passed in request_irq (), and there will be use 
>> after free problem.
>
> This needs to be explained (in details) in the commit message.

Okay.

>
>>
>>
>>>> Fixes: b11bc1ae4658 ("drm/hisilicon/hibmc: Add MSI irq getting and 
>>>> requesting for HPD")
>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>> ---
>>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 +++-------
>>>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/ 
>>>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>>> index 768b97f9e74a..4cdcc34070ee 100644
>>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>>> @@ -32,7 +32,7 @@
>>>>   DEFINE_DRM_GEM_FOPS(hibmc_fops);
>>>> -static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { 
>>>> "vblank", "hpd" };
>>>> +static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "hibmc- 
>>>> vblank", "hibmc-hpd" };
>>> Please point to the corresponding IRQ names as currently implemented in
>>> the upstream kernel.
>>
>> Ok.
>
> I was thinking in terms of IRQ lookup. You can ignore this comment (it 
> makes me wonder, how did you understand it, if you responded with Ok).
>
I thought you were suggesting naming the irq referring to the previous name.


>>
>>
>>>>   static irqreturn_t hibmc_interrupt(int irq, void *arg)
>>>>   {
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.