Re: [PATCH V2] video/aperture: match the pci device when calling sysfb_disable()

I forgot to update the patch title but it should probably be something like:

video/aperture: optionally match the device in sysfb_disable()

Alex

On Mon, Aug 19, 2024 at 1:00 PM Alex Deucher alexander.deucher@amd.com wrote:

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

Fixes: 5ae3716cfdcd ("video/aperture: Only remove sysfb on the default vga pci device") Cc: Javier Martinez Canillas javierm@redhat.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Helge Deller deller@gmx.de Cc: Sam Ravnborg sam@ravnborg.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Alex Deucher alexander.deucher@amd.com Cc: stable@vger.kernel.org

---

drivers/firmware/sysfb.c | 11 +++++++++-- drivers/of/platform.c | 2 +- drivers/video/aperture.c | 5 ++--- include/linux/sysfb.h | 4 ++-- 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index 880ffcb500887..033a044af2646 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,8 +64,12 @@ 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;
  

•   if (dev && dev != sysfb_parent_dev(si))
  

•           return;
    mutex_lock(&disable_lock);
    sysfb_unregister();
    disabled = true;
  

@@ -93,7 +100,7 @@ static __init bool sysfb_pci_dev_is_enabled(struct pci_dev *pdev) } #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 389d4ea6bfc15..ef622d41eb5b2 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 561be8feca96c..b23d85ceea104 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);
  
  

@@ -353,8 +353,7 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na 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))
  

diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h index c9cb657dad08a..bef5f06a91de6 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) { }

-- 2.46.0