linux-kernel - Re: [PATCH AUTOSEL 5.17 18/31] s390/pci: improve zpci

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: <9a0af2fec80f1b46c3ad9d80c6424e168ca2e54e.camel@linux.ibm.com>
Date:   Thu, 07 Apr 2022 10:27:37 +0200
From:   Niklas Schnelle <schnelle@...ux.ibm.com>
To:     Sasha Levin <sashal@...nel.org>, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Cc:     Matthew Rosato <mjrosato@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        gerald.schaefer@...ux.ibm.com, hca@...ux.ibm.com,
        agordeev@...ux.ibm.com, linux-s390@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 5.17 18/31] s390/pci: improve zpci_dev reference
 counting

On Wed, 2022-04-06 at 21:10 -0400, Sasha Levin wrote:
> From: Niklas Schnelle <schnelle@...ux.ibm.com>
> 
> [ Upstream commit c122383d221dfa2f41cfe5e672540595de986fde ]
> 
> Currently zpci_dev uses kref based reference counting but only accounts
> for one original reference plus one reference from an added pci_dev to
> its underlying zpci_dev. Counting just the original reference worked
> until the pci_dev reference was added in commit 2a671f77ee49 ("s390/pci:
> fix use after free of zpci_dev") because once a zpci_dev goes away, i.e.
> enters the reserved state, it would immediately get released. However
> with the pci_dev reference this is no longer the case and the zpci_dev
> may still appear in multiple availability events indicating that it was
> reserved. This was solved by detecting when the zpci_dev is already on
> its way out but still hanging around. This has however shown some light
> on how unusual our zpci_dev reference counting is.
> 
> Improve upon this by modelling zpci_dev reference counting on pci_dev.
> Analogous to pci_get_slot() increment the reference count in
> get_zdev_by_fid(). Thus all users of get_zdev_by_fid() must drop the
> reference once they are done with the zpci_dev.
> 
> Similar to pci_scan_single_device(), zpci_create_device() returns the
> device with an initial count of 1 and the device added to the zpci_list
> (analogous to the PCI bus' device_list). In turn users of
> zpci_create_device() must only drop the reference once the device is
> gone from the point of view of the zPCI subsystem, it might still be
> referenced by the common PCI subsystem though.
> 
> Reviewed-by: Matthew Rosato <mjrosato@...ux.ibm.com>
> Signed-off-by: Niklas Schnelle <schnelle@...ux.ibm.com>
> Signed-off-by: Vasily Gorbik <gor@...ux.ibm.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---

This isn't really a bug fix, as far as I'm aware the existing code
works correctly. It is just about making things more like PCI bus
reference counting and less weird. I also see some potential of the
state of things with just this commit added being confusing. That's why
there is a follow up commit 7dcfe50f58d2 ("s390/pci: rename
get_zdev_by_bus() to zdev_from_bus()") to make it more obvious when
zpci_zdev_put() is needed.

In short I'd propose to drop this patch from the stable queues.