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

Thomas Zimmermann
2023-06-07T15:15:54Z

[-- Attachment #1.1: Type: text/plain, Size: 4429 bytes --]

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

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]