linux-kernel - Re: [RESEND PATCH v1] misc: pci_endpoint_test: Fix irq

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] [day] [month] [year] [list]

Message-ID: <cba08e9c-9a91-4c55-b2f8-a24499321300@socionext.com>
Date: Mon, 20 Jan 2025 19:24:23 +0900
From: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Kishon Vijay Abraham I <kishon@...nel.org>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH v1] misc: pci_endpoint_test: Fix irq_type to convey
 the correct type

Hi Bjorn,

Thank you for your comment.

On 2025/01/17 0:04, Bjorn Helgaas wrote:
> On Thu, Jan 16, 2025 at 11:41:45AM +0900, Kunihiko Hayashi wrote:
>> There are two variables that indicate the interrupt type to be used
>> in the next test execution, global "irq_type" and test->irq_type.
>>
>> The former is referenced from pci_endpoint_test_get_irq() to preserve
>> the current type for ioctl(PCITEST_GET_IRQTYPE).
>>
>> In pci_endpoint_test_request_irq(), since this global variable is
>> referenced when an error occurs, the unintended error message is
>> displayed.
> 
> Apparently this test fails (with an error message) when it shouldn't?
> Please include the error message here. >
> "... since this global variable is referenced ..." is not quite enough
> explanation of how this causes a spurious test failure or under what
> circumstances the failure occurs.

I see. This failure path is taken if devm_request_irq() returns with
an error. The message that causes an error in the third interrupt is for
example:

   pci-endpoint-test 0000:01:00.0: Failed to request IRQ 30 for MSI 3

Before this fix, this message will show "MSI" even if the current irq
type was "legacy" or "MSI-X".

I'll add such that description in the commit.

And I discovered that not releasing interrupt correctly caused WARN()
after displaying this message. I'll fix the issue additionally.

>> And the type set in pci_endpoint_test_set_irq() isn't reflected in
>> the global "irq_type", so ioctl(PCITEST_GET_IRQTYPE) returns the
> previous
>> type. As a result, the wrong type will be displayed in "pcitest".
> 
> The global "irq_type" seems a little suspect.  Is it possible to run
> multiple tests concurrently?  If so, is this usage safe from races?

The global "irq_type" is only changed in pci_endpoint_test_set_irq().
I think this function is protected by the ioctl's mutex even if running
multiple tests.

Thank you,

---
Best Regards
Kunihiko Hayashi