Skip to content

Commit db6422d

Browse files
committed
s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails
jira LE-2349 Rebuild_History Non-Buildable kernel-4.18.0-553.40.1.el8_10 commit-author Niklas Schnelle <schnelle@linux.ibm.com> commit 4879610 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-4.18.0-553.40.1.el8_10/48796104.failed Prior to commit 0467cdd ("s390/pci: Sort PCI functions prior to creating virtual busses") the IOMMU was initialized and the device was registered as part of zpci_create_device() with the struct zpci_dev freed if either resulted in an error. With that commit this was moved into a separate function called zpci_add_device(). While this new function logs when adding failed, it expects the caller not to use and to free the struct zpci_dev on error. This difference between it and zpci_create_device() was missed while changing the callers and the incompletely initialized struct zpci_dev may get used in zpci_scan_configured_device in the error path. This then leads to a crash due to the device not being registered with the zbus. It was also not freed in this case. Fix this by handling the error return of zpci_add_device(). Since in this case the zdev was not added to the zpci_list it can simply be discarded and freed. Also make this more explicit by moving the kref_init() into zpci_add_device() and document that zpci_zdev_get()/zpci_zdev_put() must be used after adding. Cc: stable@vger.kernel.org Fixes: 0467cdd ("s390/pci: Sort PCI functions prior to creating virtual busses") Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> Signed-off-by: Heiko Carstens <hca@linux.ibm.com> (cherry picked from commit 4879610) Signed-off-by: Jonathan Maple <jmaple@ciq.com> # Conflicts: # arch/s390/pci/pci.c # arch/s390/pci/pci_event.c
1 parent dfbf357 commit db6422d

File tree

1 file changed

+184
-0
lines changed

1 file changed

+184
-0
lines changed
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails
2+
3+
jira LE-2349
4+
Rebuild_History Non-Buildable kernel-4.18.0-553.40.1.el8_10
5+
commit-author Niklas Schnelle <schnelle@linux.ibm.com>
6+
commit 48796104c864cf4dafa80bd8c2ce88f9c92a65ea
7+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
8+
Will be included in final tarball splat. Ref for failed cherry-pick at:
9+
ciq/ciq_backports/kernel-4.18.0-553.40.1.el8_10/48796104.failed
10+
11+
Prior to commit 0467cdde8c43 ("s390/pci: Sort PCI functions prior to
12+
creating virtual busses") the IOMMU was initialized and the device was
13+
registered as part of zpci_create_device() with the struct zpci_dev
14+
freed if either resulted in an error. With that commit this was moved
15+
into a separate function called zpci_add_device().
16+
17+
While this new function logs when adding failed, it expects the caller
18+
not to use and to free the struct zpci_dev on error. This difference
19+
between it and zpci_create_device() was missed while changing the
20+
callers and the incompletely initialized struct zpci_dev may get used in
21+
zpci_scan_configured_device in the error path. This then leads to
22+
a crash due to the device not being registered with the zbus. It was
23+
also not freed in this case. Fix this by handling the error return of
24+
zpci_add_device(). Since in this case the zdev was not added to the
25+
zpci_list it can simply be discarded and freed. Also make this more
26+
explicit by moving the kref_init() into zpci_add_device() and document
27+
that zpci_zdev_get()/zpci_zdev_put() must be used after adding.
28+
29+
Cc: stable@vger.kernel.org
30+
Fixes: 0467cdde8c43 ("s390/pci: Sort PCI functions prior to creating virtual busses")
31+
Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>
32+
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
33+
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
34+
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
35+
(cherry picked from commit 48796104c864cf4dafa80bd8c2ce88f9c92a65ea)
36+
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
37+
38+
# Conflicts:
39+
# arch/s390/pci/pci.c
40+
# arch/s390/pci/pci_event.c
41+
diff --cc arch/s390/pci/pci.c
42+
index 162ca752cec9,6a011d040dfe..000000000000
43+
--- a/arch/s390/pci/pci.c
44+
+++ b/arch/s390/pci/pci.c
45+
@@@ -884,10 -801,34 +885,41 @@@ struct zpci_dev *zpci_create_device(u3
46+
goto error;
47+
zdev->state = state;
48+
49+
++<<<<<<< HEAD
50+
+ kref_init(&zdev->kref);
51+
+ mutex_init(&zdev->lock);
52+
+ mutex_init(&zdev->kzdev_lock);
53+
+
54+
++=======
55+
+ mutex_init(&zdev->state_lock);
56+
+ mutex_init(&zdev->fmb_lock);
57+
+ mutex_init(&zdev->kzdev_lock);
58+
+
59+
+ return zdev;
60+
+
61+
+ error:
62+
+ zpci_dbg(0, "crt fid:%x, rc:%d\n", fid, rc);
63+
+ kfree(zdev);
64+
+ return ERR_PTR(rc);
65+
+ }
66+
+
67+
+ /**
68+
+ * zpci_add_device() - Add a previously created zPCI device to the zPCI subsystem
69+
+ * @zdev: The zPCI device to be added
70+
+ *
71+
+ * A struct zpci_dev is added to the zPCI subsystem and to a virtual PCI bus creating
72+
+ * a new one as necessary. A hotplug slot is created and events start to be handled.
73+
+ * If successful from this point on zpci_zdev_get() and zpci_zdev_put() must be used.
74+
+ * If adding the struct zpci_dev fails the device was not added and should be freed.
75+
+ *
76+
+ * Return: 0 on success, or an error code otherwise
77+
+ */
78+
+ int zpci_add_device(struct zpci_dev *zdev)
79+
+ {
80+
+ int rc;
81+
+
82+
+ zpci_dbg(1, "add fid:%x, fh:%x, c:%d\n", zdev->fid, zdev->fh, zdev->state);
83+
++>>>>>>> 48796104c864 (s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails)
84+
rc = zpci_init_iommu(zdev);
85+
if (rc)
86+
goto error;
87+
@@@ -1168,6 -1105,50 +1201,53 @@@ bool zpci_is_enabled(void
88+
return s390_pci_initialized;
89+
}
90+
91+
++<<<<<<< HEAD
92+
++=======
93+
+ static int zpci_cmp_rid(void *priv, const struct list_head *a,
94+
+ const struct list_head *b)
95+
+ {
96+
+ struct zpci_dev *za = container_of(a, struct zpci_dev, entry);
97+
+ struct zpci_dev *zb = container_of(b, struct zpci_dev, entry);
98+
+
99+
+ /*
100+
+ * PCI functions without RID available maintain original order
101+
+ * between themselves but sort before those with RID.
102+
+ */
103+
+ if (za->rid == zb->rid)
104+
+ return za->rid_available > zb->rid_available;
105+
+ /*
106+
+ * PCI functions with RID sort by RID ascending.
107+
+ */
108+
+ return za->rid > zb->rid;
109+
+ }
110+
+
111+
+ static void zpci_add_devices(struct list_head *scan_list)
112+
+ {
113+
+ struct zpci_dev *zdev, *tmp;
114+
+
115+
+ list_sort(NULL, scan_list, &zpci_cmp_rid);
116+
+ list_for_each_entry_safe(zdev, tmp, scan_list, entry) {
117+
+ list_del_init(&zdev->entry);
118+
+ if (zpci_add_device(zdev))
119+
+ kfree(zdev);
120+
+ }
121+
+ }
122+
+
123+
+ int zpci_scan_devices(void)
124+
+ {
125+
+ LIST_HEAD(scan_list);
126+
+ int rc;
127+
+
128+
+ rc = clp_scan_pci_devices(&scan_list);
129+
+ if (rc)
130+
+ return rc;
131+
+
132+
+ zpci_add_devices(&scan_list);
133+
+ zpci_bus_scan_busses();
134+
+ return 0;
135+
+ }
136+
+
137+
++>>>>>>> 48796104c864 (s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails)
138+
static int __init pci_base_init(void)
139+
{
140+
int rc;
141+
diff --cc arch/s390/pci/pci_event.c
142+
index b3961f1016ea,7f7b732b3f3e..000000000000
143+
--- a/arch/s390/pci/pci_event.c
144+
+++ b/arch/s390/pci/pci_event.c
145+
@@@ -328,6 -340,10 +328,13 @@@ static void __zpci_event_availability(s
146+
zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
147+
if (IS_ERR(zdev))
148+
break;
149+
++<<<<<<< HEAD
150+
++=======
151+
+ if (zpci_add_device(zdev)) {
152+
+ kfree(zdev);
153+
+ break;
154+
+ }
155+
++>>>>>>> 48796104c864 (s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails)
156+
} else {
157+
/* the configuration request may be stale */
158+
if (zdev->state != ZPCI_FN_STATE_STANDBY)
159+
@@@ -337,10 -353,17 +344,22 @@@
160+
zpci_scan_configured_device(zdev, ccdf->fh);
161+
break;
162+
case 0x0302: /* Reserved -> Standby */
163+
++<<<<<<< HEAD
164+
+ if (!zdev)
165+
+ zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
166+
+ else
167+
++=======
168+
+ if (!zdev) {
169+
+ zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
170+
+ if (IS_ERR(zdev))
171+
+ break;
172+
+ if (zpci_add_device(zdev)) {
173+
+ kfree(zdev);
174+
+ break;
175+
+ }
176+
+ } else {
177+
++>>>>>>> 48796104c864 (s390/pci: Fix leak of struct zpci_dev when zpci_add_device() fails)
178+
zpci_update_fh(zdev, ccdf->fh);
179+
- }
180+
break;
181+
case 0x0303: /* Deconfiguration requested */
182+
if (zdev) {
183+
* Unmerged path arch/s390/pci/pci.c
184+
* Unmerged path arch/s390/pci/pci_event.c

0 commit comments

Comments
 (0)