Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable

Geert Uytterhoeven
2023-06-07T08:48:30Z

Hi Thomas,

Thanks for your patch!

On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann <tzimmermann@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.

> 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... ;-)

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.

> 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@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?

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.

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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds