linux-kernel - Re: [PATCH 3/3] genirq: Check for trigger type mismatch in __setup

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]

Date:   Mon, 06 Jun 2022 09:49:43 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     tglx@...utronix.de, bjorn.andersson@...aro.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 3/3] genirq: Check for trigger type mismatch in __setup_irq()

On Mon, 30 May 2022 09:08:42 +0100,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org> wrote:
> 
> Currently, if the trigger type defined by the platform like DT does not
> match the driver requested trigger type, the below warning is shown
> during platform_get_irq() but only during the second time of the drive
> probe (due to probe deferral or module unload/load).
> 
> irq: type mismatch, failed to map hwirq-9 for interrupt-controller@...0000!
> 
> Consider a typical usecase of requesting an IRQ in a driver:
> 
> ```
> 	/* Assume DT has set the trigger type to IRQF_TYPE_LEVEL_HIGH */
> 
> 	q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
> 	if (q6v5->wdog_irq <= 0)
> 		return q6v5->wdog_irq;
> 
> 	ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
> 					NULL, q6v5_wdog_interrupt,
> 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> 					"q6v5 wdog", q6v5);
> 	if (ret) {
> 		dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
> 		return ret;
> 	}
> ```
> 
> For the first time probe of a driver, platform_get_irq_byname() does not
> return an error and it sets the platform requested trigger type. Then,
> request_irq() also does not check for the trigger type mismatch and sets
> the driver requested trigger type. Later if the driver gets probed again,
> platform_get_irq() throws the "type mismatch" warning and fails.
> 
> Ideally, request_irq() should throw the error during the first time itself,
> when it detects the trigger type mismatch. So let's add a check in
> __setup_irq() for checking the trigger type mismatch.

No, that's wrong. The whole point is to be able to *override* the
default that is exposed by the device tree or ACPI. We have countless
examples of that, and they cannot be broken.

If the issue exists after an unload, then it is a unload time that the
previous behaviour should be restored.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.