linux-kernel - Re: Regression: firmware/sysfb.c device path

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <-sJINpD9sbYc288gXp2-Tf5w6diSusXAP8mM8l0mnLWfIXgjuNe1LBgkqQ2indMJOehVrPUPO3UMQ-AvObgCWUW9h_TepvcNy9gnqfvoyvM=@proton.me>
Date: Mon, 15 Jul 2024 09:03:17 +0000
From: Tj <tj.iam.tj@...ton.me>
To: Thomas Zimmermann <tzimmermann@...e.de>
Cc: Marek Olsak <Marek.Olsak@....com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "1075713@...s.debian.org" <1075713@...s.debian.org>
Subject: Re: Regression: firmware/sysfb.c device path

On Monday, 15 July 2024 at 07:44, Thomas Zimmermann <tzimmermann@...e.de> wrote:

> > See hw/xfree86/fbdevhw/fbdevhw.c::fbdev_open()
> >
> > https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/hw/xfree86/fbdevhw/fbdevhw.c?ref_type=heads#L381
>
>
> Amazing debugging skills!
>
> The patch that causes the regression in X sets the PCI device as parent
> for the VESA framebuffer. That means that the PCI device won't be
> unplugged or suspended while the VESA framebuffer is still in use.
> Without, results are undefined.
>
> Therefore, could this be fixed in X' fbdev driver?

I was in two minds about this one. On the 'perfection' side I agree Xorg should not rely on a symbolic link at all - though that is easy to say; I wasn't the one that had to find a way to do what fbdevhw has to do at the time!

But the other me argues that kernel ought not to break userspace and as it is a pretty common scenario across distros that userspace stays relatively stable (sans security bugs) but kernels can and do move on quite rapidly and frequently the kernel ought to play nice here :)

With this in mind I can foresee many systems that work perfectly fine will break when only the kernel is upgraded and thus it'll have the finger of blame pointed at it.

Since this is more likely to strike in automated test harness scenarios (due to it being relatively obscure) as well there may well be circumstances where changing a userspace library immediately is extremely problematical.

I wonder if there's some half-way deprecation measure here that would allow a period of transition?

I can imagine one way would be a custom udev rule (for systemd-udevd hosts - not sure about non-udevd) that replaces the new symlink with the old - but if a distro does that it's almost as easy to patch and rebuild fbdevhw - and far more brittle.

> A fix could look like this:
>
> 1) readlink /sys/class/graphics/fb0/device/subsystem
>  2) strcmp to "bus/pci"

This works and doesn't cause a regression in v6.8.12:

~ # uname -r; grep fbdev /var/log/Xorg.0.log
6.9.7-amd64
[    31.376] (==) Matched fbdev as autoconfigured driver 2
[    31.377] (II) LoadModule: "fbdev"
[    31.377] (II) Loading /usr/lib/xorg/modules/drivers/fbdev_drv.so
[    31.377] (II) Module fbdev: vendor="X.Org Foundation"
[    31.377] (II) FBDEV: driver for framebuffer: fbdev
[    31.410] fbdev trace: FBDevPciProbe()
[    31.410] (II) Loading sub module "fbdevhw"
[    31.410] (II) LoadModule: "fbdevhw"
[    31.410] (II) Loading /usr/lib/xorg/modules/libfbdevhw.so
[    31.410] (II) Module fbdevhw: vendor="X.Org Foundation"
[    31.410] fbdev trace: FBDevPciProbe() return
[    31.410] (WW) Falling back to old probe method for fbdev
[    31.410] fbdev trace: FBDevProbe()
[    31.410] (II) Loading sub module "fbdevhw"
[    31.410] (II) LoadModule: "fbdevhw"
[    31.410] (II) Loading /usr/lib/xorg/modules/libfbdevhw.so
[    31.410] (II) Module fbdevhw: vendor="X.Org Foundation"
[    31.410] fbdev: FBDevProbe() for() numDevSection=0
[    31.410] fbdev: FBDevProbe() isPci0 isISA=0
[    31.410] fbdev: FBDevProbe() calling fbdevHWProbe(NULL, '(null)', NULL)
[    31.410] (II) fbdev_open(scrnIndex=-1, dev=(null), namep=(nil))
[    31.410] (II) fbdev_open() using dev from env FRAMEBUFFER=(null)
[    31.410] (II) fbdev_open() using default dev=/dev/fb0
[    31.410] (II) fbdev_open() sysfs_path=/sys/class/graphics/fb0/device/subsystem
[    31.410] (II) fbdev_open() buf=../../../../bus/platform
[    31.410] (II) fbdev_open() returning file descriptor 11
[    31.410] fbdev trace: FBDevProbe() fbdevHWProbe()
[    31.410] fbdev trace: FBDevProbe() else xf86ClaimFbSlot()
[    31.410] fbdev trace: FBDevProbe() return
[    31.410] (II) UnloadModule: "fbdev"
[    31.410] (II) UnloadSubModule: "fbdevhw"
[    31.410] fbdev: PreInit 0
[    31.410] (II) FBDEV(0): fbdev_open(scrnIndex=0, dev=(null), namep=(nil))
[    31.410] (II) FBDEV(0): fbdev_open() using dev from env FRAMEBUFFER=(null)
[    31.410] (II) FBDEV(0): fbdev_open() using default dev=/dev/fb0
[    31.410] (II) FBDEV(0): fbdev_open() sysfs_path=/sys/class/graphics/fb0/device/subsystem
[    31.410] (II) FBDEV(0): fbdev_open() buf=../../../../bus/platform
[    31.410] (II) FBDEV(0): fbdev_open() returning file descriptor 12

~ # uname -r; grep fbdev /var/log/Xorg.0.log
6.8.12-amd64
[    14.225] (==) Matched fbdev as autoconfigured driver 2
[    14.225] (II) LoadModule: "fbdev"
[    14.225] (II) Loading /usr/lib/xorg/modules/drivers/fbdev_drv.so
[    14.225] (II) Module fbdev: vendor="X.Org Foundation"
[    14.225] (II) FBDEV: driver for framebuffer: fbdev
[    14.252] fbdev trace: FBDevPciProbe()
[    14.252] (II) Loading sub module "fbdevhw"
[    14.252] (II) LoadModule: "fbdevhw"
[    14.252] (II) Loading /usr/lib/xorg/modules/libfbdevhw.so
[    14.252] (II) Module fbdevhw: vendor="X.Org Foundation"
[    14.253] fbdev trace: FBDevPciProbe() return
[    14.253] (WW) Falling back to old probe method for fbdev
[    14.253] fbdev trace: FBDevProbe()
[    14.253] (II) Loading sub module "fbdevhw"
[    14.253] (II) LoadModule: "fbdevhw"
[    14.253] (II) Loading /usr/lib/xorg/modules/libfbdevhw.so
[    14.253] (II) Module fbdevhw: vendor="X.Org Foundation"
[    14.253] fbdev: FBDevProbe() for() numDevSection=0
[    14.253] fbdev: FBDevProbe() isPci0 isISA=0
[    14.253] fbdev: FBDevProbe() calling fbdevHWProbe(NULL, '(null)', NULL)
[    14.253] (II) fbdev_open(scrnIndex=-1, dev=(null), namep=(nil))
[    14.253] (II) fbdev_open() using dev from env FRAMEBUFFER=(null)
[    14.253] (II) fbdev_open() using default dev=/dev/fb0
[    14.253] (II) fbdev_open() sysfs_path=/sys/class/graphics/fb0/device/subsystem
[    14.253] (II) fbdev_open() buf=../../../bus/platform
[    14.253] (II) fbdev_open() returning file descriptor 11
[    14.253] fbdev trace: FBDevProbe() fbdevHWProbe()
[    14.253] fbdev trace: FBDevProbe() else xf86ClaimFbSlot()
[    14.253] fbdev trace: FBDevProbe() return
[    14.253] (II) UnloadModule: "fbdev"
[    14.253] (II) UnloadSubModule: "fbdevhw"
[    14.253] fbdev: PreInit 0
[    14.253] (II) FBDEV(0): fbdev_open(scrnIndex=0, dev=(null), namep=(nil))
[    14.253] (II) FBDEV(0): fbdev_open() using dev from env FRAMEBUFFER=(null)
[    14.253] (II) FBDEV(0): fbdev_open() using default dev=/dev/fb0
[    14.253] (II) FBDEV(0): fbdev_open() sysfs_path=/sys/class/graphics/fb0/device/subsystem
[    14.253] (II) FBDEV(0): fbdev_open() buf=../../../bus/platform
[    14.253] (II) FBDEV(0): fbdev_open() returning file descriptor 12

I'll propose this patch in Debian Xorg for Unstable/Testing but not sure if it'll be accepted for Stable.