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 <tzimmerm...@suse.de> 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. It

I'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 <tzimmerm...@suse.de>

--- 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 framebuffer

affect

+         console.
+
  config FB_DDC
         tristate
         depends on FB

Gr{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

Reply via email to