Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Am 11.06.23 um 18:37 schrieb Sam Ravnborg: Hi Thomas, On Mon, Jun 05, 2023 at 04:48:12PM +0200, Thomas Zimmermann 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. s/ a// 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 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. 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. Should that be a TODO in gpu/todo.rst? Otherwise the amount of people knowing about this is very close to 1. As an alternative add a TODO to each Kconfig file. Signed-off-by: Thomas Zimmermann --- drivers/staging/fbtft/Kconfig| 1 + drivers/video/fbdev/Kconfig | 12 + drivers/video/fbdev/core/Makefile| 7 +++--- drivers/video/fbdev/core/fb_internal.h | 32 drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- include/linux/fb.h | 2 ++ 6 files changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig index 4d29e8c1014e..5dda3c65a38e 100644 --- a/drivers/staging/fbtft/Kconfig +++ b/drivers/staging/fbtft/Kconfig @@ -2,6 +2,7 @@ menuconfig FB_TFT tristate "Support for small TFT LCD display modules" depends on FB && SPI + depends on FB_DEVICE depends on GPIOLIB || COMPILE_TEST select FB_SYS_FILLRECT select FB_SYS_COPYAREA diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 6df9bd09454a..48d9a14f889c 100644 --- 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" +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 + console. tabs to spaces to indent the above correct. + config FB_DDC tristate depends on FB @@ -1545,6 +1554,7 @@ config FB_3DFX_I2C config FB_VOODOO1 tristate "3Dfx Voodoo Graphics (sst1) support" depends on FB && PCI + depends on FB_DEVICE select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1862,6 +1872,7 @@ config FB_SH_MOBILE_LCDC tristate "SuperH Mobile LCDC framebuffer support" depends on FB && HAVE_CLK && HAS_IOMEM depends on SUPERH || ARCH_RENESAS || COMPILE_TEST + depends on FB_DEVICE select FB_SYS_FILLRECT select FB_SYS_COPYAREA select FB_SYS_IMAGEBLIT @@ -1930,6 +1941,7 @@ config FB_SMSCUFX config FB_UDL tristate "Displaylink USB Framebuffer support" depends on FB && USB + depends on FB_DEVICE select FB_MODE_HELPERS select FB_SYS_FILLRECT select FB_SYS_COPYAREA diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile index 125d24f50c36..d5e8772620f8 100644 --- a/drivers/video/fbdev/core/Makefile +++ b/drivers/video/fbdev/core/Makefile @@ -2,12 +2,13 @@ obj-$(CONFIG_FB_NOTIFY) += fb_notify.o obj-$(CONFIG_FB) += fb.o fb-y := fb_backlight.o \ - fb_devfs.o \ fb_info.o \ - fb_procfs.o \ - fbmem.o fbmon.o fbcmap.o fbsysfs.o \ + fbmem.o fbmon.o fbcmap.o \ modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o +fb-$(CONFIG_FB_DEVICE)+= fb_devfs.o \ + fb_procfs.o \ + fbsysfs.o Maybe change this to one line to avoid '\'? ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) fb-y+= fbcon.o bitblit.o softcursor.o diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h index 0b43c0cd5096..b8a28466db79 100644 ---
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Sam Am 11.06.23 um 18:37 schrieb Sam Ravnborg: Hi Thomas, On Mon, Jun 05, 2023 at 04:48:12PM +0200, Thomas Zimmermann 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. s/ a// 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 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. 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. Should that be a TODO in gpu/todo.rst? Otherwise the amount of people knowing about this is very close to 1. Ha! Yes, I'll add a TODO item. Good idea. Best regards Thomas As an alternative add a TODO to each Kconfig file. Signed-off-by: Thomas Zimmermann --- drivers/staging/fbtft/Kconfig| 1 + drivers/video/fbdev/Kconfig | 12 + drivers/video/fbdev/core/Makefile| 7 +++--- drivers/video/fbdev/core/fb_internal.h | 32 drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- include/linux/fb.h | 2 ++ 6 files changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig index 4d29e8c1014e..5dda3c65a38e 100644 --- a/drivers/staging/fbtft/Kconfig +++ b/drivers/staging/fbtft/Kconfig @@ -2,6 +2,7 @@ menuconfig FB_TFT tristate "Support for small TFT LCD display modules" depends on FB && SPI + depends on FB_DEVICE depends on GPIOLIB || COMPILE_TEST select FB_SYS_FILLRECT select FB_SYS_COPYAREA diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 6df9bd09454a..48d9a14f889c 100644 --- 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" +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 + console. tabs to spaces to indent the above correct. + config FB_DDC tristate depends on FB @@ -1545,6 +1554,7 @@ config FB_3DFX_I2C config FB_VOODOO1 tristate "3Dfx Voodoo Graphics (sst1) support" depends on FB && PCI + depends on FB_DEVICE select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1862,6 +1872,7 @@ config FB_SH_MOBILE_LCDC tristate "SuperH Mobile LCDC framebuffer support" depends on FB && HAVE_CLK && HAS_IOMEM depends on SUPERH || ARCH_RENESAS || COMPILE_TEST + depends on FB_DEVICE select FB_SYS_FILLRECT select FB_SYS_COPYAREA select FB_SYS_IMAGEBLIT @@ -1930,6 +1941,7 @@ config FB_SMSCUFX config FB_UDL tristate "Displaylink USB Framebuffer support" depends on FB && USB + depends on FB_DEVICE select FB_MODE_HELPERS select FB_SYS_FILLRECT select FB_SYS_COPYAREA diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile index 125d24f50c36..d5e8772620f8 100644 --- a/drivers/video/fbdev/core/Makefile +++ b/drivers/video/fbdev/core/Makefile @@ -2,12 +2,13 @@ obj-$(CONFIG_FB_NOTIFY) += fb_notify.o obj-$(CONFIG_FB) += fb.o fb-y := fb_backlight.o \ - fb_devfs.o \ fb_info.o \ - fb_procfs.o \ - fbmem.o fbmon.o fbcmap.o fbsysfs.o \ + fbmem.o fbmon.o fbcmap.o \ modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o +fb-$(CONFIG_FB_DEVICE)+= fb_devfs.o \ + fb_procfs.o \ + fbsysfs.o Maybe change this to one line to avoid '\'? ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) fb-y+= fbcon.o bitblit.o softcursor.o diff --git a/drivers/video/fbdev/core/fb_internal.h
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas, On Mon, Jun 05, 2023 at 04:48:12PM +0200, Thomas Zimmermann 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. s/ a// > > 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 > 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. > > 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. Should that be a TODO in gpu/todo.rst? Otherwise the amount of people knowing about this is very close to 1. As an alternative add a TODO to each Kconfig file. > > Signed-off-by: Thomas Zimmermann > --- > drivers/staging/fbtft/Kconfig| 1 + > drivers/video/fbdev/Kconfig | 12 + > drivers/video/fbdev/core/Makefile| 7 +++--- > drivers/video/fbdev/core/fb_internal.h | 32 > drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- > include/linux/fb.h | 2 ++ > 6 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig > index 4d29e8c1014e..5dda3c65a38e 100644 > --- a/drivers/staging/fbtft/Kconfig > +++ b/drivers/staging/fbtft/Kconfig > @@ -2,6 +2,7 @@ > menuconfig FB_TFT > tristate "Support for small TFT LCD display modules" > depends on FB && SPI > + depends on FB_DEVICE > depends on GPIOLIB || COMPILE_TEST > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > index 6df9bd09454a..48d9a14f889c 100644 > --- 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" > +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 > + console. tabs to spaces to indent the above correct. > + > config FB_DDC > tristate > depends on FB > @@ -1545,6 +1554,7 @@ config FB_3DFX_I2C > config FB_VOODOO1 > tristate "3Dfx Voodoo Graphics (sst1) support" > depends on FB && PCI > + depends on FB_DEVICE > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -1862,6 +1872,7 @@ config FB_SH_MOBILE_LCDC > tristate "SuperH Mobile LCDC framebuffer support" > depends on FB && HAVE_CLK && HAS_IOMEM > depends on SUPERH || ARCH_RENESAS || COMPILE_TEST > + depends on FB_DEVICE > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > select FB_SYS_IMAGEBLIT > @@ -1930,6 +1941,7 @@ config FB_SMSCUFX > config FB_UDL > tristate "Displaylink USB Framebuffer support" > depends on FB && USB > + depends on FB_DEVICE > select FB_MODE_HELPERS > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > diff --git a/drivers/video/fbdev/core/Makefile > b/drivers/video/fbdev/core/Makefile > index 125d24f50c36..d5e8772620f8 100644 > --- a/drivers/video/fbdev/core/Makefile > +++ b/drivers/video/fbdev/core/Makefile > @@ -2,12 +2,13 @@ > obj-$(CONFIG_FB_NOTIFY) += fb_notify.o > obj-$(CONFIG_FB) += fb.o > fb-y := fb_backlight.o \ > - fb_devfs.o \ > fb_info.o \ > - fb_procfs.o \ > - fbmem.o fbmon.o fbcmap.o fbsysfs.o \ > + fbmem.o fbmon.o fbcmap.o \ > modedb.o fbcvt.o fb_cmdline.o > fb_io_fops.o > fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o > +fb-$(CONFIG_FB_DEVICE)+= fb_devfs.o \ > + fb_procfs.o \ > + fbsysfs.o Maybe change this to one line to avoid '\'? > > ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) > fb-y += fbcon.o bitblit.o softcursor.o > diff --git a/drivers/video/fbdev/core/fb_internal.h >
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Thomas Zimmermann writes: [...] >> >> So with FB_CORE, you can have default y if you have a real fbdev driver, >> and default n if you have only DRM drivers. >> All this discussion about FB_CORE is unrelated to your series though and that is covered by enabling CONFIG_FB_DEVICE. I think that there's value on a FB_CORE option to allow disabling all the fbdev drivers with a single option but still keep DRM_FBDEV_EMULATION enabled. But I can repost my old series on top of this patch-set once it lands. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas,
On Fri, Jun 9, 2023 at 1:04 PM Thomas Zimmermann wrote:
> Am 09.06.23 um 11:14 schrieb Geert Uytterhoeven:
> [...]
> >
> >>> What do you think low-end embedded devices with an out-of-tree[*]
> >>> fbdev driver are using?
> >>
> >> And those do not count either. IIRC Android used to be built on top of
> >> fbdev devices. I'm not sure if they have moved to DRM by now. But
> >> embedded uses dedicated kernels and kernel configs. It's easy for them
> >> to set FB_DEVICE=y. We're not going to take away the fbdev device
> >> entirely.
> >
> > The point is that we do not suddenly disable functionality that users
> > may depend on. While "make oldconfig" will show users the new
> > FB_DEVICE question, (and hopefully they'll notice), "make olddefconfig"
> > and "make _defconfig" won't, possibly causing regressions.
> > Without a suitable default, you should IMHO at least update all
> > defconfigs that enable any fbdev drivers.
>
> Didn't I already say that we should make it "default y" if that's
> preferable in practice?
OK, sorry, I seem to have missed that part.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi
Am 09.06.23 um 11:14 schrieb Geert Uytterhoeven:
[...]
What do you think low-end embedded devices with an out-of-tree[*]
fbdev driver are using?
And those do not count either. IIRC Android used to be built on top of
fbdev devices. I'm not sure if they have moved to DRM by now. But
embedded uses dedicated kernels and kernel configs. It's easy for them
to set FB_DEVICE=y. We're not going to take away the fbdev device entirely.
The point is that we do not suddenly disable functionality that users
may depend on. While "make oldconfig" will show users the new
FB_DEVICE question, (and hopefully they'll notice), "make olddefconfig"
and "make _defconfig" won't, possibly causing regressions.
Without a suitable default, you should IMHO at least update all
defconfigs that enable any fbdev drivers.
Didn't I already say that we should make it "default y" if that's
preferable in practice?
Best regards
Thomas
I guess you do remember the fall-out from f611b1e7624ccdbd ("drm:
Avoid circular dependencies for CONFIG_FB"), after which lots of defconfigs
had to gain CONFIG_FB=y?
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)
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Geert Uytterhoeven writes: >> * fbdev drivers + DRM fbdev emulation + fbdev user-space > > "fbdev drivers + fbdev user-space", I assume? > Right, I meant to also include "only fbdev drivers + fbdev user-space" but forgot :) >> * only DRM drivers without fbdev emulation nor fbdev user-space (your series) > > Thomas' series includes fbdev emulation here, for the text console. > Yes, I meant fbdev emulation for user-space. Probably should had included fbcon in the options too... But what I tried to say is that we need all combination of DRM drivers, fbdev drivers, DRM emulation for fbcon and emulation for fbdev user-space. And I think that Thomas' series + a FB_CORE as you suggested and done in my old series should be enough to have that. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Javier,
On Fri, Jun 9, 2023 at 11:59 AM Javier Martinez Canillas
wrote:
> Thomas Zimmermann writes:
> >>> I'd also question the argument that there's even fbdev userspace out
> >>> there. It was never popular in it's heyday and definitely hasn't
> >>> improved since then. Even the 3 people who still ask for fbdev support
> >>
> >> There's X.org, DirectFB, SDL, ...
> >
> > None of these examples has a dependency on fbdev. They can freely switch
> > backends and have moved to DRM. Anything program utilizing these
> > examples has no dependency on fbdev either.
> >
> > When I say "userspace" in this context, it's the one old program that
> > supports nothing but fbdev. TBH I'm having problems to come up with
> > examples.
> >
>
> I personally have two real world examples that can give to you :)
>
> 1) I've a IoT device at home that has a bunch of sensors (temperatury,
>humidity, etc) and a SSD1306 display panel to report that. It just
>has small fbdev program to print that info. I could probably port
>to KMS but didn't feel like it. Found a fbdev program that I could
>modify and got the job done.
>
> 2) I built a portable retro console for my kids, that uses a ST7735R
>LCD panel. The software I use is https://www.retroarch.com/ which
>uses fbdev by default (I believe that supports a KMS mode but out
>of the box it works with fbdev and that's better tested by them.
>
> So even when I'm not interested and don't want to enable any of the
> fbdev drivers, I want to use the ssd130x and st7735r DRM drivers and
> the DRM fbdev emulation layer.
>
> In other words, there's real use cases for supporting fbdev programs
> with DRM drivers. Now, I agree with this patch-set and probably will
> disable the user-space fbdev interface in Fedora, but on my embedded
> projects I will probably keep it enabled.
>
> That's why I think that we should support the following combinations:
>
> * fbdev drivers + DRM fbdev emulation + fbdev user-space
"fbdev drivers + fbdev user-space", I assume?
> * only DRM drivers without fbdev emulation nor fbdev user-space (your series)
Thomas' series includes fbdev emulation here, for the text console.
> * only DRM fbdev emulation + fbdev user-space enabled (FB_CORE)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Thomas Zimmermann writes: Hello Thomas, > Hi > [...] >>> I'd also question the argument that there's even fbdev userspace out >>> there. It was never popular in it's heyday and definitely hasn't >>> improved since then. Even the 3 people who still ask for fbdev support >> >> There's X.org, DirectFB, SDL, ... > > None of these examples has a dependency on fbdev. They can freely switch > backends and have moved to DRM. Anything program utilizing these > examples has no dependency on fbdev either. > > When I say "userspace" in this context, it's the one old program that > supports nothing but fbdev. TBH I'm having problems to come up with > examples. > I personally have two real world examples that can give to you :) 1) I've a IoT device at home that has a bunch of sensors (temperatury, humidity, etc) and a SSD1306 display panel to report that. It just has small fbdev program to print that info. I could probably port to KMS but didn't feel like it. Found a fbdev program that I could modify and got the job done. 2) I built a portable retro console for my kids, that uses a ST7735R LCD panel. The software I use is https://www.retroarch.com/ which uses fbdev by default (I believe that supports a KMS mode but out of the box it works with fbdev and that's better tested by them. So even when I'm not interested and don't want to enable any of the fbdev drivers, I want to use the ssd130x and st7735r DRM drivers and the DRM fbdev emulation layer. In other words, there's real use cases for supporting fbdev programs with DRM drivers. Now, I agree with this patch-set and probably will disable the user-space fbdev interface in Fedora, but on my embedded projects I will probably keep it enabled. That's why I think that we should support the following combinations: * fbdev drivers + DRM fbdev emulation + fbdev user-space * only DRM drivers without fbdev emulation nor fbdev user-space (your series) * only DRM fbdev emulation + fbdev user-space enabled (FB_CORE) -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas, On Fri, Jun 9, 2023 at 10:00 AM Thomas Zimmermann wrote: > Am 09.06.23 um 09:29 schrieb Geert Uytterhoeven: > > On Fri, Jun 9, 2023 at 9:09 AM Thomas Zimmermann > > wrote: > >> Am 08.06.23 um 01:07 schrieb Javier Martinez Canillas: > >>> Geert Uytterhoeven writes: > On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann > wrote: > > Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven: > >> On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann > >> wrote: > >>> --- 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. > > Defaulting to "n" (the default) means causing regressions when these > few people use an existing defconfig. > > >>> > >>> Having "dfault y if !DRM" makes sense to me. I guess is a corner case but > >>> at least it won't silently be disabled for users that only want fbdev as > >>> Geert mentioned. > >> > >> IMHO the rational behind such conditionals are mostly what "we make up > >> here in the discussion", but not something based on real-world feedback. > >> So I'd strongly prefer a clear n or y setting here. > >> > >>> > >>> I wouldn't call it a regression though, because AFAIK the Kconfig options > >>> are not a stable API ? > >> > >> IIRC in the past there have been concerns about changing Kconfig > >> defaults. If we go with "default n", we'd apparently do something similar. > >> > >>> > >> 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. > >>> > >>> Funny that you mention because it's exactly what I attempted in the past: > >>> > >>> https://lore.kernel.org/all/20210827100531.1578604-1-javi...@redhat.com/T/#u > >>> > > > > 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. > > "Default y" does not mean that you cannot disable FB_DEVICE, so > you are not forced to bring back the device? > >>> > >>> I think we can have both to make the kernel more configurable: > >>> > >>> 1) Allow to only disable fbdev user-space APIs (/dev/fb?, /proc/fb, etc), > >>> which is what the series is doing with the new FB_DEVICE config > >>> symbol. > >>> > >>> 2) Allow to disable all "native" fbdev drivers and only keep the DRM fbdev > >>> emulation layer. That's what my series attempted to do with the > >>> FB_CORE > >>> Kconfig symbol. > >>> > >>> I believe that there are use cases for both, for example as Thomas' said > >>> many distros are disabling all the fbdev drivers and their user-space only > >>> requires DRM/KMS, so makes sense to not expose any fbdev uAPI at all. > >>> > >>> But may be that other users want the opposite, they have an old user-space > >>> that requires fbdev, but is running on newer hardware that only have a DRM > >>> driver. So they will want DRM fbdev emulation but none fbdev driver at > >>> all. > >>> > >>> That's why I think that FB_DEVICE and FB_CORE are complementary and we can > >>> support any combination of the two, if you agree there are uses for > >>> either. > >> > >> I still don't understand the value of such an extra compile-time option? > >>Either you have fbdev userspace, then you want the device; or you > >> don't then it's better to disable it entirely. I don't see much of a > >> difference between DRM and fbdev drivers here. > > > > If you have DRM and are running a Linux desktop, you are probably > > using DRM userspace. > > If you have fbdev, and are using graphics, you have no choice but > > using an fbdev userspace. > > > > So with FB_CORE, you can have default y if you have a real fbdev driver, > > and default n if you have only DRM drivers. > > > >> I'd also question the argument that there's even fbdev userspace out > >> there. It was never popular in it's heyday and definitely hasn't > >> improved since then. Even the 3 people who still ask for fbdev support > > > > There's X.org, DirectFB, SDL, ... > > None of these examples has a dependency on fbdev. They can freely switch > backends and have moved to DRM.
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Am 09.06.23 um 09:29 schrieb Geert Uytterhoeven: Hi Thomas, On Fri, Jun 9, 2023 at 9:09 AM Thomas Zimmermann wrote: Am 08.06.23 um 01:07 schrieb Javier Martinez Canillas: Geert Uytterhoeven writes: On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann wrote: Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven: On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann wrote: --- 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. Defaulting to "n" (the default) means causing regressions when these few people use an existing defconfig. Having "dfault y if !DRM" makes sense to me. I guess is a corner case but at least it won't silently be disabled for users that only want fbdev as Geert mentioned. IMHO the rational behind such conditionals are mostly what "we make up here in the discussion", but not something based on real-world feedback. So I'd strongly prefer a clear n or y setting here. I wouldn't call it a regression though, because AFAIK the Kconfig options are not a stable API ? IIRC in the past there have been concerns about changing Kconfig defaults. If we go with "default n", we'd apparently do something similar. 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. Funny that you mention because it's exactly what I attempted in the past: https://lore.kernel.org/all/20210827100531.1578604-1-javi...@redhat.com/T/#u 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. "Default y" does not mean that you cannot disable FB_DEVICE, so you are not forced to bring back the device? I think we can have both to make the kernel more configurable: 1) Allow to only disable fbdev user-space APIs (/dev/fb?, /proc/fb, etc), which is what the series is doing with the new FB_DEVICE config symbol. 2) Allow to disable all "native" fbdev drivers and only keep the DRM fbdev emulation layer. That's what my series attempted to do with the FB_CORE Kconfig symbol. I believe that there are use cases for both, for example as Thomas' said many distros are disabling all the fbdev drivers and their user-space only requires DRM/KMS, so makes sense to not expose any fbdev uAPI at all. But may be that other users want the opposite, they have an old user-space that requires fbdev, but is running on newer hardware that only have a DRM driver. So they will want DRM fbdev emulation but none fbdev driver at all. That's why I think that FB_DEVICE and FB_CORE are complementary and we can support any combination of the two, if you agree there are uses for either. I still don't understand the value of such an extra compile-time option? Either you have fbdev userspace, then you want the device; or you don't then it's better to disable it entirely. I don't see much of a difference between DRM and fbdev drivers here. If you have DRM and are running a Linux desktop, you are probably using DRM userspace. If you have fbdev, and are using graphics, you have no choice but using an fbdev userspace. So with FB_CORE, you can have default y if you have a real fbdev driver, and default n if you have only DRM drivers. I'd also question the argument that there's even fbdev userspace out there. It was never popular in it's heyday and definitely hasn't improved since then. Even the 3 people who still ask for fbdev support There's X.org, DirectFB, SDL, ... None of these examples has a dependency on fbdev. They can freely switch backends and have moved to DRM. Anything program utilizing these examples has no dependency on fbdev either. When I say "userspace" in this context, it's the one old program that supports nothing but fbdev. TBH I'm having problems to come up with examples. What do you think low-end embedded devices with an out-of-tree[*] fbdev driver are using? And those do not count either. IIRC Android used to be built on top of fbdev devices. I'm not sure if they have moved to DRM by now. But embedded uses dedicated kernels and kernel configs. It's easy for them to set FB_DEVICE=y. We're not going to take away the fbdev device entirely. [*] There's been a moratorium on new fbdev drivers for about a decade.
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas, On Fri, Jun 9, 2023 at 9:09 AM Thomas Zimmermann wrote: > Am 08.06.23 um 01:07 schrieb Javier Martinez Canillas: > > Geert Uytterhoeven writes: > >> On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann > >> wrote: > >>> Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven: > On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann > wrote: > > --- 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. > >> > >> Defaulting to "n" (the default) means causing regressions when these > >> few people use an existing defconfig. > >> > > > > Having "dfault y if !DRM" makes sense to me. I guess is a corner case but > > at least it won't silently be disabled for users that only want fbdev as > > Geert mentioned. > > IMHO the rational behind such conditionals are mostly what "we make up > here in the discussion", but not something based on real-world feedback. > So I'd strongly prefer a clear n or y setting here. > > > > > I wouldn't call it a regression though, because AFAIK the Kconfig options > > are not a stable API ? > > IIRC in the past there have been concerns about changing Kconfig > defaults. If we go with "default n", we'd apparently do something similar. > > > > 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. > > > > Funny that you mention because it's exactly what I attempted in the past: > > > > https://lore.kernel.org/all/20210827100531.1578604-1-javi...@redhat.com/T/#u > > > >>> > >>> 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. > >> > >> "Default y" does not mean that you cannot disable FB_DEVICE, so > >> you are not forced to bring back the device? > > > > I think we can have both to make the kernel more configurable: > > > > 1) Allow to only disable fbdev user-space APIs (/dev/fb?, /proc/fb, etc), > > which is what the series is doing with the new FB_DEVICE config symbol. > > > > 2) Allow to disable all "native" fbdev drivers and only keep the DRM fbdev > > emulation layer. That's what my series attempted to do with the FB_CORE > > Kconfig symbol. > > > > I believe that there are use cases for both, for example as Thomas' said > > many distros are disabling all the fbdev drivers and their user-space only > > requires DRM/KMS, so makes sense to not expose any fbdev uAPI at all. > > > > But may be that other users want the opposite, they have an old user-space > > that requires fbdev, but is running on newer hardware that only have a DRM > > driver. So they will want DRM fbdev emulation but none fbdev driver at all. > > > > That's why I think that FB_DEVICE and FB_CORE are complementary and we can > > support any combination of the two, if you agree there are uses for either. > > I still don't understand the value of such an extra compile-time option? > Either you have fbdev userspace, then you want the device; or you > don't then it's better to disable it entirely. I don't see much of a > difference between DRM and fbdev drivers here. If you have DRM and are running a Linux desktop, you are probably using DRM userspace. If you have fbdev, and are using graphics, you have no choice but using an fbdev userspace. So with FB_CORE, you can have default y if you have a real fbdev driver, and default n if you have only DRM drivers. > I'd also question the argument that there's even fbdev userspace out > there. It was never popular in it's heyday and definitely hasn't > improved since then. Even the 3 people who still ask for fbdev support There's X.org, DirectFB, SDL, ... What do you think low-end embedded devices with an out-of-tree[*] fbdev driver are using? [*] There's been a moratorium on new fbdev drivers for about a decade. > here only seem to care about the performance of the framebuffer console, > but never about userspace. Unless you go for heavy graphics and 3D, a simple GUI with some buttons and text requires less performance than scrolling a full-screen graphical text console... > So I'd like to propose a different solution: on top of the current
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Geert and Javier Am 08.06.23 um 01:07 schrieb Javier Martinez Canillas: Geert Uytterhoeven writes: Hello Geert and Thomas, Hi Thomas, On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann wrote: Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven: On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann wrote: --- 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. Defaulting to "n" (the default) means causing regressions when these few people use an existing defconfig. Having "dfault y if !DRM" makes sense to me. I guess is a corner case but at least it won't silently be disabled for users that only want fbdev as Geert mentioned. IMHO the rational behind such conditionals are mostly what "we make up here in the discussion", but not something based on real-world feedback. So I'd strongly prefer a clear n or y setting here. I wouldn't call it a regression though, because AFAIK the Kconfig options are not a stable API ? IIRC in the past there have been concerns about changing Kconfig defaults. If we go with "default n", we'd apparently do something similar. 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. Funny that you mention because it's exactly what I attempted in the past: https://lore.kernel.org/all/20210827100531.1578604-1-javi...@redhat.com/T/#u 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. "Default y" does not mean that you cannot disable FB_DEVICE, so you are not forced to bring back the device? I think we can have both to make the kernel more configurable: 1) Allow to only disable fbdev user-space APIs (/dev/fb?, /proc/fb, etc), which is what the series is doing with the new FB_DEVICE config symbol. 2) Allow to disable all "native" fbdev drivers and only keep the DRM fbdev emulation layer. That's what my series attempted to do with the FB_CORE Kconfig symbol. I believe that there are use cases for both, for example as Thomas' said many distros are disabling all the fbdev drivers and their user-space only requires DRM/KMS, so makes sense to not expose any fbdev uAPI at all. But may be that other users want the opposite, they have an old user-space that requires fbdev, but is running on newer hardware that only have a DRM driver. So they will want DRM fbdev emulation but none fbdev driver at all. That's why I think that FB_DEVICE and FB_CORE are complementary and we can support any combination of the two, if you agree there are uses for either. I still don't understand the value of such an extra compile-time option? Either you have fbdev userspace, then you want the device; or you don't then it's better to disable it entirely. I don't see much of a difference between DRM and fbdev drivers here. I'd also question the argument that there's even fbdev userspace out there. It was never popular in it's heyday and definitely hasn't improved since then. Even the 3 people who still ask for fbdev support here only seem to care about the performance of the framebuffer console, but never about userspace. So I'd like to propose a different solution: on top of the current patchset, let's make an fbdev module option that enables the device. If CONFIG_FB_DEVICE has been enabled, the option would switch the functionality on and off. A Kconfig option would set the default. With such a setup, distributions can disable the fbdev device by default. And the few users with the odd system that has fbdev userspace can still enable the fbdev device at boot time. Best regards Thomas -- 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) OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Geert Uytterhoeven writes: Hello Geert and Thomas, > Hi Thomas, > > On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann wrote: >> Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven: >> > On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann >> > wrote: >> >> --- 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. > > Defaulting to "n" (the default) means causing regressions when these > few people use an existing defconfig. > Having "dfault y if !DRM" makes sense to me. I guess is a corner case but at least it won't silently be disabled for users that only want fbdev as Geert mentioned. I wouldn't call it a regression though, because AFAIK the Kconfig options are not a stable API ? >> > 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. Funny that you mention because it's exactly what I attempted in the past: https://lore.kernel.org/all/20210827100531.1578604-1-javi...@redhat.com/T/#u >> >> 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. > > "Default y" does not mean that you cannot disable FB_DEVICE, so > you are not forced to bring back the device? > I think we can have both to make the kernel more configurable: 1) Allow to only disable fbdev user-space APIs (/dev/fb?, /proc/fb, etc), which is what the series is doing with the new FB_DEVICE config symbol. 2) Allow to disable all "native" fbdev drivers and only keep the DRM fbdev emulation layer. That's what my series attempted to do with the FB_CORE Kconfig symbol. I believe that there are use cases for both, for example as Thomas' said many distros are disabling all the fbdev drivers and their user-space only requires DRM/KMS, so makes sense to not expose any fbdev uAPI at all. But may be that other users want the opposite, they have an old user-space that requires fbdev, but is running on newer hardware that only have a DRM driver. So they will want DRM fbdev emulation but none fbdev driver at all. That's why I think that FB_DEVICE and FB_CORE are complementary and we can support any combination of the two, if you agree there are uses for either. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas,
On Wed, Jun 7, 2023 at 5:15 PM Thomas Zimmermann wrote:
> Am 07.06.23 um 10:48 schrieb Geert Uytterhoeven:
> > On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann
> > wrote:
> >> --- 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.
Defaulting to "n" (the default) means causing regressions when these
few people use an existing defconfig.
> > 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.
"Default y" does not mean that you cannot disable FB_DEVICE, so
you are not forced to bring back the device?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
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 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
--- 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)
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas,
Thanks for your patch!
On Mon, Jun 5, 2023 at 4:48 PM Thomas Zimmermann 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
> --- 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 -- ge...@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
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20230605]
[cannot apply to drm-misc/drm-misc-next lee-backlight/for-backlight-next
staging/staging-testing staging/staging-next staging/staging-linus linus/master
lee-backlight/for-backlight-fixes v6.4-rc5 v6.4-rc4 v6.4-rc3 v6.4-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/backlight-bd6107-Compare-against-struct-fb_info-device/20230605-225002
base: next-20230605
patch link:
https://lore.kernel.org/r/20230605144812.15241-31-tzimmermann%40suse.de
patch subject: [PATCH 30/30] fbdev: Make support for userspace interfaces
configurable
config: powerpc-randconfig-r036-20230605
(https://download.01.org/0day-ci/archive/20230606/202306060547.528padrx-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project
4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
#
https://github.com/intel-lab-lkp/linux/commit/d309f36af8a1ee56ef56e54287ca6d2bf7d239de
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Thomas-Zimmermann/backlight-bd6107-Compare-against-struct-fb_info-device/20230605-225002
git checkout d309f36af8a1ee56ef56e54287ca6d2bf7d239de
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1
O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1
O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/video/fbdev/mb862xx/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202306060547.528padrx-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c:793:15: error: no member named
>> 'dev' in 'struct fb_info'
dev_dbg(fbi->dev, "%s release\n", fbi->fix.id);
~~~ ^
include/linux/dev_printk.h:163:26: note: expanded from macro 'dev_dbg'
dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:129:22: note: expanded from macro 'dev_printk'
_dev_printk(level, dev, fmt, ##__VA_ARGS__);\
^~~
1 error generated.
vim +793 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 785
3a2ab02ddfacb0 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c Uwe Kleine-König
2023-03-19 786 static void of_platform_mb862xx_remove(struct platform_device
*ofdev)
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 787 {
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 788struct fb_info *fbi = dev_get_drvdata(>dev);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 789struct mb862xxfb_par *par = fbi->par;
28f65c11f2ffb3 drivers/video/mb862xx/mb862xxfbdrv.c Joe Perches
2011-06-09 790resource_size_t res_size = resource_size(par->res);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 791unsigned long reg;
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 792
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 @793dev_dbg(fbi->dev, "%s release\n", fbi->fix.id);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 794
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 795/* display off */
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 796reg = inreg(disp, GC_DCM1);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 797reg &= ~(GC_DCM01_DEN | GC_DCM01_L0E);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 798outreg(disp, GC_DCM1, reg);
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 799
17a1217e12d8c8 drivers/video/mb862xx/mb862xxfb.c Anatolij Gustschin
2008-11-06 800/* disable
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
On Mon, Jun 05, 2023 at 04:48:12PM +0200, Thomas Zimmermann 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 > 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. > > 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 Acked-by: Greg Kroah-Hartman
