Re: [PATCH v2 00/38] fbdev: Make userspace interfaces optional
From: Sam Ravnborg <hidden>
Date: 2023-06-12 15:56:48
Also in:
dri-devel, linux-omap, linux-sh, linux-staging, lkml
Hi Thomas, Nice series, quite some preparations. On Mon, Jun 12, 2023 at 04:07:38PM +0200, Thomas Zimmermann wrote:
Add the new config option FB_DEVICE. If enabled, fbdev provides
traditional userspace interfaces in devfs, sysfs and procfs, such
as /dev/fb0 or /proc/fb.
Modern Linux distrobutions have adopted DRM drivers for graphics
output and use fbdev only for the kernel's framebuffer console.
Userspace has also moved on, with no new fbdev code being written
and existing support being removed.
OTOH, fbdev provides userspace a way of accessing kernel or I/O
memory, which might compromise the system's security. See the recent
commit c8687694bb1f ("drm/fbdev-generic: prohibit potential
out-of-bounds access") for an example. Disabling fbdev userspace
interfaces is therefore a useful feature to limit unnecessary
exposure of fbdev code to processes of low privilegues.
Patches 1 to 31 fix various bugs and issues in fbdev-related code.
In most cases the code uses the fbdev device where it should use
the Linux hardware device or something else. Most of these patches
fix existing problems and should therefore be considered in any case.
Patches 32 to 37 refactor the fbdev core code. The patches move
support for backlights, sysfs, procfs and devfs into separate files
and hide it behind simple interfaces. These changes will allow to
easily build the userspace support conditionally.
Patch 38 introduces the config option FB_DEVICE and adapts the fbdev
core to support it. The field struct fb_info.dev is now optional,
hence the name of the config option.
Tested on simpledrm and i915, including the device handover.
Future directions: With the support for disabling fbdev userspace
interfaces in place, it will be possible to make most fbdev drivers'
file-I/O code in struct fb_ops optional as well.
v2:
* fix fsl-diu-fb and sh7760fb
* split backlight patches
* set 'default y' for FB_CONFIG
* minor fixes and corrections
Thomas Zimmermann (38):
backlight/bd6107: Compare against struct fb_info.device
backlight/bd6107: Rename struct bd6107_platform_data.fbdev to 'dev'
backlight/gpio_backlight: Compare against struct fb_info.device
backlight/gpio_backlight: Rename field 'fbdev' to 'dev'
backlight/lv5207lp: Compare against struct fb_info.device
backlight/lv5207lp: Rename struct lv5207lp_platform_data.fbdev to
'dev'
fbdev/atyfb: Reorder backlight and framebuffer init/cleanup
fbdev/atyfb: Use hardware device as backlight parent
fbdev/aty128fb: Reorder backlight and framebuffer init/cleanup
fbdev/aty128fb: Use hardware device as backlight parent
fbdev/broadsheetfb: Call device_remove_file() with hardware device
fbdev/ep93xx-fb: Output messages with fb_info() and fb_err()
fbdev/ep93xx-fb: Do not assign to struct fb_info.dev
fbdev/fsl-diu-fb: Output messages with fb_*() helpers
fbdev/mb862xxfb: Output messages with fb_dbg()
fbdev/metronomefb: Use hardware device for dev_err()
fbdev/nvidiafb: Reorder backlight and framebuffer init/cleanup
fbdev/nvidiafb: Use hardware device as backlight parent
fbdev/pxa168fb: Do not assign to struct fb_info.dev
fbdev/radeonfb: Reorder backlight and framebuffer cleanup
fbdev/radeonfb: Use hardware device as backlight parent
fbdev/rivafb: Reorder backlight and framebuffer init/cleanup
fbdev/rivafb: Use hardware device as backlight parent
fbdev/sh7760fb: Use fb_dbg() in sh7760fb_get_color_info()
fbdev/sh7760fb: Output messages with fb_dbg()
fbdev/sh7760fb: Use hardware device with dev_() output during probe
fbdev/sm501fb: Output message with fb_err()
fbdev/tdfxfb: Set i2c adapter parent to hardware deviceThe above are all: Reviewed-by: Sam Ravnborg <redacted>
fbdev/smscufx: Detect registered fb_info from refcount
I did not try to understand the code, so others must review.
fbdev/ep93xx-fb: Alloc DMA memory from hardware device fbdev/sh7760fb: Alloc DMA memory from hardware device
This smells like bug-fixes, and I do not see what impact the change has. So again, someone else needs to provide review here. The rest are already reviewed or got a dedicated reply. Sam