Nouveau - Jan 2016 - [PATCH] device/tegra: fix uninitialized IRQ number

Am Montag, den 25.01.2016, 18:44 +0900 schrieb Alexandre
Courbot:
> nvkm_device_tegra_new initializes the irq member of the Tegra device
> to -1 in order to signal that it is uninitialized. However,
> nvkm_device_tegra_fini tests it against 0 to check whether an IRQ has
> been allocated or not. This leads to free_irq being called on -1
> during
> device initialization.
> 
The convention in other parts of the Linux kernel is that IRQ number 0
means unallocated/invalid IRQ. So I think it is the initialization to
-1 that should be fixed instead.
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
>  drm/nouveau/nvkm/engine/device/tegra.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drm/nouveau/nvkm/engine/device/tegra.c
> b/drm/nouveau/nvkm/engine/device/tegra.c
> index 7f8a42721eb2..0a22439fd9ae 100644
> --- a/drm/nouveau/nvkm/engine/device/tegra.c
> +++ b/drm/nouveau/nvkm/engine/device/tegra.c
> @@ -195,9 +195,9 @@ static void
>  nvkm_device_tegra_fini(struct nvkm_device *device, bool suspend)
>  {
>  	struct nvkm_device_tegra *tdev = nvkm_device_tegra(device);
> -	if (tdev->irq) {
> +	if (tdev->irq != -1) {
>  		free_irq(tdev->irq, tdev);
> -		tdev->irq = 0;
> +		tdev->irq = -1;
>  	};
>  }
>  

On Wed, Jan 27, 2016 at 5:44 AM, Lucas Stach <dev at lynxeye.de>
wrote:
> Am Montag, den 25.01.2016, 18:44 +0900 schrieb Alexandre Courbot:
>> nvkm_device_tegra_new initializes the irq member of the Tegra device
>> to -1 in order to signal that it is uninitialized. However,
>> nvkm_device_tegra_fini tests it against 0 to check whether an IRQ has
>> been allocated or not. This leads to free_irq being called on -1
>> during
>> device initialization.
>>
> The convention in other parts of the Linux kernel is that IRQ number 0
> means unallocated/invalid IRQ. So I think it is the initialization to
> -1 that should be fixed instead.
Uh, that sounds unsafe. Thanks for spotting this, will resend.

nvkm_device_tegra_new initializes the irq member of the Tegra device
to -1 in order to signal that it is uninitialized. However,
nvkm_device_tegra_fini tests it against 0 to check whether an IRQ has
been allocated or not. This leads to free_irq being called on -1 during
device initialization.

Fix this by using 0 as the uninitialized value everywhere.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
v2: use 0 as the uninitialized value, as suggested by Lucas.

 drm/nouveau/nvkm/engine/device/tegra.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drm/nouveau/nvkm/engine/device/tegra.c
b/drm/nouveau/nvkm/engine/device/tegra.c
index 7f8a42721eb2..6d89416f0bc1 100644
--- a/drm/nouveau/nvkm/engine/device/tegra.c
+++ b/drm/nouveau/nvkm/engine/device/tegra.c
@@ -255,7 +255,6 @@ nvkm_device_tegra_new(const struct nvkm_device_tegra_func
*func,
 	*pdevice = &tdev->device;
 	tdev->func = func;
 	tdev->pdev = pdev;
-	tdev->irq = -1;
 
 	tdev->vdd = devm_regulator_get(&pdev->dev, "vdd");
 	if (IS_ERR(tdev->vdd))
-- 
2.7.0