Re: [PATCH V3] video/aperture: optionally match the device in sysfb_disable()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Am 21.08.24 um 21:11 schrieb Alex Deucher:
In aperture_remove_conflicting_pci_devices(), we currently only
call sysfb_disable() on vga class devices.  This leads to the
following problem when the pimary device is not VGA compatible:

1. A PCI device with a non-VGA class is the boot display
2. That device is probed first and it is not a VGA device so
    sysfb_disable() is not called, but the device resources
    are freed by aperture_detach_platform_device()
3. Non-primary GPU has a VGA class and it ends up calling sysfb_disable()
4. NULL pointer dereference via sysfb_disable() since the resources
    have already been freed by aperture_detach_platform_device() when
    it was called by the other device.

Fix this by passing a device pointer to sysfb_disable() and checking
the device to determine if we should execute it or not.

v2: Fix build when CONFIG_SCREEN_INFO is not set
v3: Move device check into the mutex
     Drop primary variable in aperture_remove_conflicting_pci_devices()
     Drop __init on pci sysfb_pci_dev_is_enabled()

Fixes: 5ae3716cfdcd ("video/aperture: Only remove sysfb on the default vga pci device")
Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
Cc: Helge Deller <deller@xxxxxx>
Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx

This change also makes aperture_remove_conflicting_pci_devices() much cleaner. Thanks a lot. Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/firmware/sysfb.c | 19 +++++++++++++------
  drivers/of/platform.c    |  2 +-
  drivers/video/aperture.c | 11 +++--------
  include/linux/sysfb.h    |  4 ++--
  4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 880ffcb50088..ac4680dc463f 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -39,6 +39,8 @@ static struct platform_device *pd;
  static DEFINE_MUTEX(disable_lock);
  static bool disabled;
+static struct device *sysfb_parent_dev(const struct screen_info *si);
+
  static bool sysfb_unregister(void)
  {
  	if (IS_ERR_OR_NULL(pd))
@@ -52,6 +54,7 @@ static bool sysfb_unregister(void)
/**
   * sysfb_disable() - disable the Generic System Framebuffers support
+ * @dev:	the device to check if non-NULL
   *
   * This disables the registration of system framebuffer devices that match the
   * generic drivers that make use of the system framebuffer set up by firmware.
@@ -61,17 +64,21 @@ static bool sysfb_unregister(void)
   * Context: The function can sleep. A @disable_lock mutex is acquired to serialize
   *          against sysfb_init(), that registers a system framebuffer device.
   */
-void sysfb_disable(void)
+void sysfb_disable(struct device *dev)
  {
+	struct screen_info *si = &screen_info;
+
  	mutex_lock(&disable_lock);
-	sysfb_unregister();
-	disabled = true;
+	if (!dev || dev == sysfb_parent_dev(si)) {
+		sysfb_unregister();
+		disabled = true;
+	}
  	mutex_unlock(&disable_lock);
  }
  EXPORT_SYMBOL_GPL(sysfb_disable);
#if defined(CONFIG_PCI)
-static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
+static bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
  {
  	/*
  	 * TODO: Try to integrate this code into the PCI subsystem
@@ -87,13 +94,13 @@ static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
  	return true;
  }
  #else
-static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
+static bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev)
  {
  	return false;
  }
  #endif
-static __init struct device *sysfb_parent_dev(const struct screen_info *si)
+static struct device *sysfb_parent_dev(const struct screen_info *si)
  {
  	struct pci_dev *pdev;
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 389d4ea6bfc1..ef622d41eb5b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -592,7 +592,7 @@ static int __init of_platform_default_populate_init(void)
  			 * This can happen for example on DT systems that do EFI
  			 * booting and may provide a GOP handle to the EFI stub.
  			 */
-			sysfb_disable();
+			sysfb_disable(NULL);
  			of_platform_device_create(node, NULL, NULL);
  			of_node_put(node);
  		}
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 561be8feca96..2b5a1e666e9b 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -293,7 +293,7 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
  	 * ask for this, so let's assume that a real driver for the display
  	 * was already probed and prevent sysfb to register devices later.
  	 */
-	sysfb_disable();
+	sysfb_disable(NULL);
aperture_detach_devices(base, size); @@ -346,15 +346,10 @@ EXPORT_SYMBOL(__aperture_remove_legacy_vga_devices);
   */
  int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
  {
-	bool primary = false;
  	resource_size_t base, size;
  	int bar, ret = 0;
- if (pdev == vga_default_device())
-		primary = true;
-
-	if (primary)
-		sysfb_disable();
+	sysfb_disable(&pdev->dev);
for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
  		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
@@ -370,7 +365,7 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
  	 * that consumes the VGA framebuffer I/O range. Remove this
  	 * device as well.
  	 */
-	if (primary)
+	if (pdev == vga_default_device())
  		ret = __aperture_remove_legacy_vga_devices(pdev);
return ret;
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index c9cb657dad08..bef5f06a91de 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -58,11 +58,11 @@ struct efifb_dmi_info {
#ifdef CONFIG_SYSFB -void sysfb_disable(void);
+void sysfb_disable(struct device *dev);
#else /* CONFIG_SYSFB */ -static inline void sysfb_disable(void)
+static inline void sysfb_disable(struct device *dev)
  {
  }

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux