Hi Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven:
Hi Thomas, Thanks for your patch! On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:Add Kconfig option CONFIG_FB_DEVICE and make the virtual fbdev device optional. If the new option has not been selected, fbdev does not create a files in devfs or sysfs. Most modern Linux systems run a DRM-based graphics stack that uses the kernel's framebuffer console, but has otherwise deprecated fbdev support. Yet fbdev userspace interfaces are still present. The option makes it possible to use the fbdev subsystem as console implementation without support for userspace. This closes potential entry points to manipulate kernel or I/O memory via framebuffers. ItI'd leave out the part about manipulating kernel memory, as that's a driver bug, if possible.
The driver/core distinction is somewhat fuzzy: the recent bug with OOB access was introduced accidentally in shared helper code within DRM.
And whenever I want to modify the fbdev code, I have to start bugfixing first. Just look at this patchset. A good number of the patches are bugfixes. Driver or not, I no longer trust any of the fbdev code to get anything right.
also prevents the execution of driver code via ioctl or sysfs, both of which might allow malicious software to exploit bugs in the fbdev code.Of course disabling ioctls reduces the attack surface, and this is not limited to fbdev... ;-)
Other subsystems should do the same where possible. The specific problem with DRM-plus-fbdev is that the fbdev device doesn't provide any additional value. It's too limited in functionality (even by fbdev standards), a possible source for bugs, and today's userspace wants DRM functionality.
I'm wondering if it would be worthwhile to optionally provide a more limited userspace API for real fbdev drivers: 1. No access to MMIO, only to the mapped frame buffer, 2. No driver-specific ioctls, only the standard ones.
That issue is only relevant to fbdev drivers and would be a separate patchset. My concern lies with the current distributions, which don't need the fbdev device and shouldn't provide it for the given reasons.
A small number of fbdev drivers require struct fbinfo.dev to be initialized, usually for the support of sysfs interface. Make these drivers depend on FB_DEVICE. They can later be fixed if necessary. Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>--- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -57,6 +57,15 @@ config FIRMWARE_EDID combination with certain motherboards and monitors are known to suffer from this problem. +config FB_DEVICE + bool "Provide legacy /dev/fb* device"Perhaps "default y if !DRM", although that does not help for a mixed drm/fbdev kernel build?
We could simply set it to "default y". But OTOH is it worth making it a default? Distributions will set it to the value they need/want. The very few people that build their own kernels to get certain fbdev drivers will certainly be able to enable the option by hand as well.
Or reserve "FB" for real fbdev drivers, and introduce a new FB_CORE, to be selected by both FB and DRM_FBDEV_EMULATION? Then FB_DEVICE can depend on FB_CORE, and default to y if FB.
That wouldn't work. In Tumbleweed, we still have efifb and vesafb enabled under certain conditions; merely for the kernel console. We'd have to enable CONFIG_FB, which would bring back the device.
Best regards Thomas
+ depends on FB + help + Say Y here if you want the legacy /dev/fb* device file. It's + only required if you have userspace programs that depend on + fbdev for graphics output. This does not effect the framebufferaffect+ console. + config FB_DDC tristate depends on FBGr{oetje,eeting}s, Geert
-- 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)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature