linux-kernel - Re: post 2.6.26 requires pciehp_slot_with

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: <20080731154723.GA26577@ldl.fc.hp.com>
Date:	Thu, 31 Jul 2008 09:47:23 -0600
From:	Alex Chiang <achiang@...com>
To:	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>
Cc:	Matthew Wilcox <matthew@....cx>,
	Pierre Ossman <drzeus-list@...eus.cx>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	LKML <linux-kernel@...r.kernel.org>, linux-pci@...r.kernel.org
Subject: Re: post 2.6.26 requires pciehp_slot_with_bus

* Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>:
> Thank you for patches, Alex-san!
>
> I've reviewed those patches and tested them on my ia64 machine
> that have both shpc and pcie hotplug slots. Your patch looks
> good.

Thank you for reviewing and testing.

> As you mentioned, we are considering the problem also from the
> view point of slot detection. But I think your patch is needed
> regardless of that because there might be platforms whose slots
> are detected properly but firmware assigns the physical slot
> number wrongly. I think Alex's patch should go to mainline.

That is a good point.

> P.S.: I found a possible improvement, though it is not a big
> problem and we don't not need to fix it soon. I'd like to tell
> you about it just in case. Current pci_hp_register() checks if
> name is duplicated first, before checking if another hotplug
> driver is already registered to the slot. So, if shpchp/pciehp
> driver tries to register hotplug slot that is already registered
> by the other hotplug driver (e.g. acpiphp) with the same name,
> shpchp/pciehp driver will do as follows:
>
> (1) shpchp/pciehp call pci_hp_register()
> (2) pci_hp_register() returns -EEXIST
> (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
> (4) pci_hp_register() returns -EBUSY
>
> if pci_hp_register() checked if another hotplug driver is already
> registered first, step (2) and (3) could be removed.

Thanks, that seems pretty easy to do.

Would you mind testing this patch as well? You should probably
apply it on top of the other two patches to see how all three
patches interact.

Thanks!

/ac


From: Alex Chiang <achiang@...80g5.kio>
Subject: [PATCH] PCI hotplug: check for claimed slot before duplicate named slot

Kenji Kaneshige observes that:

If shpchp/pciehp driver tries to register hotplug slot that is
already registered by the other hotplug driver (e.g. acpiphp) with
the same name, shpchp/pciehp driver will do as follows:

(1) shpchp/pciehp call pci_hp_register()
(2) pci_hp_register() returns -EEXIST
(3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
(4) pci_hp_register() returns -EBUSY

If pci_hp_register() checked if another hotplug driver is already
registered first, step (2) and (3) could be removed.

This patch does not prevent the *same* driver from attempting
to register multiple slots with the same name (on systems with
broken firmware). For that situation, we still need to detect
a name collision and return -EEXIST if so.

Signed-off-by: Alex Chiang <achiang@...com>
---
 drivers/pci/hotplug/pci_hotplug_core.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 5f85b1b..9c379b6 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -568,10 +568,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 		return -EINVAL;
 	}
 
-	/* Check if we have already registered a slot with the same name. */
-	if (get_slot_from_name(slot->name))
-		return -EEXIST;
-
 	/*
 	 * No problems if we call this interface from both ACPI_PCI_SLOT
 	 * driver and call it here again. If we've already created the
@@ -587,6 +583,12 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 		return -EBUSY;
 	}
 
+	/* Check if we have already registered a slot with the same name. */
+	if (get_slot_from_name(slot->name)) {
+		pci_destroy_slot(pci_slot);
+		return -EEXIST;
+	}
+
 	slot->pci_slot = pci_slot;
 	pci_slot->hotplug = slot;
 
@@ -609,7 +611,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
 	kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
 	dbg("Added slot %s to the list\n", slot->name);
 
-
 	return result;
 }
 
-- 
1.6.0.rc0.g95f8

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/