linux-kernel - Re: [PATCH] PCI: Fix devres regression in pci

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]

Message-ID: <9529b8012b1a1573316d65727f231f5cf54d0315.camel@redhat.com>
Date: Thu, 25 Jul 2024 17:21:39 +0200
From: Philipp Stanner <pstanner@...hat.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, Krzysztof
 Wilczyński
	 <kwilczynski@...nel.org>, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Damien Le Moal <dlemoal@...nel.org>
Subject: Re: [PATCH] PCI: Fix devres regression in pci_intx()

On Thu, 2024-07-25 at 07:26 -0700, Christoph Hellwig wrote:
> Can we please fix this to never silently redirect to a manager
> version

It is not the fix or the recent changes (which the fix is for) to PCI
devres that is doing that. pci_intx() has been "silently redirect[ing]
to a managed version" since 15 years.

The changes merged into v6.11 attempted to keep this behavior exactly
identical as a preparation for later cleanups. The fix here only
corrects the position of the redirection to where the "crazy devres
voodoo" had always been:

void pci_intx(struct pci_dev *pdev, int enable)
{
	u16 pci_command, new;

	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);

	if (enable)
		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
	else
		new = pci_command | PCI_COMMAND_INTX_DISABLE;

	if (new != pci_command) {
		struct pci_devres *dr;

		pci_write_config_word(pdev, PCI_COMMAND, new);

		/* voodoo_begin */
		dr = find_pci_dr(pdev);
		if (dr && !dr->restore_intx) {
			dr->restore_intx = 1;
			dr->orig_intx = !enable;
		}
		/* voodoo_end */
	}
}
EXPORT_SYMBOL_GPL(pci_intx);

> and add a proper pcim_intx instead 

That has already been done. pcim_intx() sits in drivers/pci/devres.c

> and use that where people actually
> want to use the crazy devres voodoo instead?  Doing this silently
> underneath will always create problems.

That's precisely what all my work is all about. The hybrid nature of
pci_intx(), pci_set_mwi() and all pci*request*() functions needs to be
removed.

However, that will take us some while, because the APIs are partly
ossificated and every user that relies on implicit crazy devres voodoo
has to be identified and then ported to *explicit* half-crazy devres
voodoo.

More details here:
https://lore.kernel.org/all/20240613115032.29098-1-pstanner@redhat.com/

P.

> 
>