linux-kernel - Re: [PATCH v4 00/15] PCI: let the core manage slot names

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: <20081009041905.GE4668@ldl.fc.hp.com>
Date:	Wed, 8 Oct 2008 22:19:05 -0600
From:	Alex Chiang <achiang@...com>
To:	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>
Cc:	jbarnes@...tuousgeek.org, kristen.c.accardi@...el.com,
	matthew@....cx, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 00/15] PCI: let the core manage slot names

* Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>:
> Hi Alex-san,
>
> I have some ideas and made some patches for comments I sent about
> your [PATCH v4 02/15] and [PATCH v4 03/15]. Please take a look.
> There are three patches.

Hi Kenji-san,

Thanks for doing this work.

I tested your patches, but found a problem with refcounting in
your 02/03, and the slot directories in sysfs remained, even
after rmmod of all drivers.

I decided that things are getting too complicated with all these
new interfaces, so I got rid of them and updated the
pci_create_slot API instead, to take a 'rename' parameter. That
way, creating a slot and overriding its name can become an atomic
operation.

That should make the race conditions go away, and the code is
much easier to understand as well.

>  - [01/03] Sample patch for [PATCH v4 02/15]

This is a good patch by itself. I think you should submit it to
Jesse. I did not need it after reworking to my new design.

>  - [02/03] Sample patch for [PATCH v4 03/15]
>    NOTE:This doesn't target the comment about changing exported
>         symbol name.

I had to stare at this patch for a long time to understand it,
and finally saw that you were changing the rename logic to detect
if we were trying to rename an existing slot.

Unfortunately, it had some problem with the refcounting, and in
this scenario:

	- pci_slot loaded
	- fakephp dup_slots=1 loaded
	- pci_slot unloaded

The slots claimed by pci_slot (but _not_ by fakephp) were never
released.

By the time I got this far, I was already thinking about a
redesign, so I did not try and debug further...

>  - [03/03] Sample patch for [PATCH v4 14/15]
>    This is needed because above two patches make your [PATCH v4
>    14/15] can not be applied.

Not needed, since I re-designed the approach.

> Note: I made those patches as replacement of your corresponding
> ones. So those patches are NOT for applying on top your original
> patches.

Again, thank you very much for all the review and hard work, and
sorry for causing so much churn. :-/

I'll be sending out the new patch series shortly.

/ac

--
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/