Message ID | 20241107-epc_rfc-v2-1-da5b6a99a66f@quicinc.com |
---|---|
State | New |
Headers | show |
Series | PCI: endpoint: fix bugs for both API pci_epc_destroy() and pci_epc_remove_epf() | expand |
On Thu, Nov 07, 2024 at 08:53:08AM +0800, Zijun Hu wrote: > From: Zijun Hu <quic_zijuhu@quicinc.com> > > pci_epc_destroy() invokes pci_bus_release_domain_nr() to release domain_nr > ID, but the invocation has below 2 faults: > > - The later accesses device @epc->dev which has been kfree()ed by previous > device_unregister(), namely, it is a UAF issue. > > - The later frees the domain_nr ID into @epc->dev, but the ID is actually > allocated from @epc->dev.parent, so it will destroy domain_nr IDA. > > Fix by freeing the ID to @epc->dev.parent before unregistering @epc->dev. > > The file(s) affected are shown below since they indirectly use the API. > drivers/pci/controller/cadence/pcie-cadence-ep.c > drivers/pci/controller/dwc/pcie-designware-ep.c > drivers/pci/controller/pcie-rockchip-ep.c > drivers/pci/controller/pcie-rcar-ep.c No need to mention the callers. > > Fixes: 0328947c5032 ("PCI: endpoint: Assign PCI domain number for endpoint controllers") > Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> > Cc: Jingoo Han <jingoohan1@gmail.com> > Cc: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Heiko Stuebner <heiko@sntech.de> > Cc: stable@vger.kernel.org > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> Good catch! (not sure how I messed up in first place). Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/pci/endpoint/pci-epc-core.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index 17f007109255..bcc9bc3d6df5 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -837,11 +837,10 @@ EXPORT_SYMBOL_GPL(pci_epc_bus_master_enable_notify); > void pci_epc_destroy(struct pci_epc *epc) > { > pci_ep_cfs_remove_epc_group(epc->group); > - device_unregister(&epc->dev); > - > #ifdef CONFIG_PCI_DOMAINS_GENERIC > - pci_bus_release_domain_nr(&epc->dev, epc->domain_nr); > + pci_bus_release_domain_nr(epc->dev.parent, epc->domain_nr); > #endif > + device_unregister(&epc->dev); > } > EXPORT_SYMBOL_GPL(pci_epc_destroy); > > > -- > 2.34.1 >
On 11/12/2024 3:03 PM, Manivannan Sadhasivam wrote: > On Thu, Nov 07, 2024 at 08:53:08AM +0800, Zijun Hu wrote: >> From: Zijun Hu <quic_zijuhu@quicinc.com> >> >> pci_epc_destroy() invokes pci_bus_release_domain_nr() to release domain_nr >> ID, but the invocation has below 2 faults: >> >> - The later accesses device @epc->dev which has been kfree()ed by previous >> device_unregister(), namely, it is a UAF issue. >> >> - The later frees the domain_nr ID into @epc->dev, but the ID is actually >> allocated from @epc->dev.parent, so it will destroy domain_nr IDA. >> >> Fix by freeing the ID to @epc->dev.parent before unregistering @epc->dev. >> >> The file(s) affected are shown below since they indirectly use the API. >> drivers/pci/controller/cadence/pcie-cadence-ep.c >> drivers/pci/controller/dwc/pcie-designware-ep.c >> drivers/pci/controller/pcie-rockchip-ep.c >> drivers/pci/controller/pcie-rcar-ep.c > > No need to mention the callers. > thank you Manivannan for code review. good suggestions, i will take them for further similar patches.(^^) >> >> Fixes: 0328947c5032 ("PCI: endpoint: Assign PCI domain number for endpoint controllers") >> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> >> Cc: Jingoo Han <jingoohan1@gmail.com> >> Cc: Marek Vasut <marek.vasut+renesas@gmail.com> >> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >> Cc: Shawn Lin <shawn.lin@rock-chips.com> >> Cc: Heiko Stuebner <heiko@sntech.de> >> Cc: stable@vger.kernel.org >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > > Good catch! (not sure how I messed up in first place). > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > - Mani > >> --- [snip] >> >
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c index 17f007109255..bcc9bc3d6df5 100644 --- a/drivers/pci/endpoint/pci-epc-core.c +++ b/drivers/pci/endpoint/pci-epc-core.c @@ -837,11 +837,10 @@ EXPORT_SYMBOL_GPL(pci_epc_bus_master_enable_notify); void pci_epc_destroy(struct pci_epc *epc) { pci_ep_cfs_remove_epc_group(epc->group); - device_unregister(&epc->dev); - #ifdef CONFIG_PCI_DOMAINS_GENERIC - pci_bus_release_domain_nr(&epc->dev, epc->domain_nr); + pci_bus_release_domain_nr(epc->dev.parent, epc->domain_nr); #endif + device_unregister(&epc->dev); } EXPORT_SYMBOL_GPL(pci_epc_destroy);