# \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - Subsystems: FRAMEBUFFER CONSOLE, FRAMEBUFFER LAYER, DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, DRM DRIVERS, DRM DRIVERS AND MISC GPU PATCHES, FRAMEBUFFER CORE, STAGING SUBSYSTEM, THE REST - Maintainers: Helge Deller, Thomas Zimmermann, Lyude Paul, Danilo Krummrich, David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard, Greg Kroah-Hartman, Linus Torvalds - Status: LANDED (04cfcc7ab358 ยท 2018-09-11 12:11) - Landed in mainline as 04cfcc7ab358 on 2018-09-11. - List: linux-fbdev - 8 messages, 3 authors - Page: https://ratatoskr.run/linux-fbdev/2018/08/5361306/t ## \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - From: Daniel Vetter \ - Date: 2018-08-22 08:54:17 - Message: https://ratatoskr.run/linux-fbdev/2018/08/5361306.md ```text This was only added for the drm's fbdev emulation support, so that it would try harder to show the Oops. Unfortunately this never really worked reliably, and in practice ended up pushing the real Oops off the screen due to plentyfull locking, sleep-while-atomic and other issues. So we removed all that support from the fbdev emulation a while back. Aside: We've also removed the kgdb support, for similar reasons. Since it's such a small patch I figured I don't split this up into the usual 3-phase removal. Cc: Ben Skeggs Cc: Bartlomiej Zolnierkiewicz Cc: Greg Kroah-Hartman Cc: Hans de Goede Cc: Daniel Vetter Cc: Alexander Kapshuk Cc: Kees Cook Cc: Thierry Reding Cc: David Lechner Cc: Cc: Signed-off-by: Daniel Vetter --- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 1 - drivers/staging/vboxvideo/vbox_fb.c | 3 +-- drivers/video/fbdev/core/fbcon.c | 1 - include/linux/fb.h | 4 ---- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 844498c4267c..20a260887be3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -379,7 +379,6 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_COPYAREA | FBINFO_HWACCEL_FILLRECT | FBINFO_HWACCEL_IMAGEBLIT; - info->flags |= FBINFO_CAN_FORCE_OUTPUT; info->fbops = &nouveau_fbcon_sw_ops; info->fix.smem_start = fb->nvbo->bo.mem.bus.base + fb->nvbo->bo.mem.bus.offset; diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c index 43c39eca4ae1..034f8ffa8f20 100644 --- a/drivers/staging/vboxvideo/vbox_fb.c +++ b/drivers/staging/vboxvideo/vbox_fb.c @@ -155,8 +155,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, * The last flag forces a mode set on VT switches even if the kernel * does not think it is needed. */ - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT | - FBINFO_MISC_ALWAYS_SETPAR; + info->flags = FBINFO_DEFAULT | FBINFO_MISC_ALWAYS_SETPAR; info->fbops = &vboxfb_ops; /* diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 5fb156bdcf4e..2b5bb52b1798 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1104,7 +1104,6 @@ static void fbcon_init(struct vc_data *vc, int init) if (p->userfont) charcnt = FNTCHARCNT(p->fontdata); - vc->vc_panic_force_write = !!(info->flags & FBINFO_CAN_FORCE_OUTPUT); vc->vc_can_do_color = (fb_get_color_depth(&info->var, &info->fix)!=1); vc->vc_complement_mask = vc->vc_can_do_color ? 0x7700 : 0x0800; if (charcnt = 256) { diff --git a/include/linux/fb.h b/include/linux/fb.h index aa74a228bb92..fa8c6f9c9c3a 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -457,10 +457,6 @@ struct fb_tile_ops { */ #define FBINFO_BE_MATH 0x100000 -/* report to the VT layer that this fb driver can accept forced console - output like oopses */ -#define FBINFO_CAN_FORCE_OUTPUT 0x200000 - struct fb_info { atomic_t count; int node; -- 2.18.0 ``` ## \[PATCH 3/4\] fbdev: Add FBINFO\_HIDE\_SMEM\_START flag - From: Daniel Vetter \ - Date: 2018-08-22 08:54:32 - Message: https://ratatoskr.run/linux-fbdev/2018/08/5361309.md ```text DRM drivers really, really, really don't want random userspace to share buffer behind it's back, bypassing the dma-buf buffer sharing machanism. For that reason we've ruthlessly rejected any IOCTL exposing the physical address of any graphics buffer. Unfortunately fbdev comes with that built-in. We could just set smem_start to 0, but that means we'd have to hand-roll our own fb_mmap implementation. For good reasons many drivers do that, but smem_start/length is still super convenient. Hence instead just stop the leak in the ioctl, to keep fb mmap working as-is. A second patch will set this flag for all drm drivers. Cc: Bartlomiej Zolnierkiewicz Cc: Kees Cook Cc: Daniel Vetter Cc: Signed-off-by: Daniel Vetter --- drivers/video/fbdev/core/fbmem.c | 4 ++++ include/linux/fb.h | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 609438d2465b..549d0f86fcf3 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1116,6 +1116,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, if (!lock_fb_info(info)) return -ENODEV; fix = info->fix; + if (info->flags & FBINFO_HIDE_SMEM_START) + fix.smem_start = 0; unlock_fb_info(info); ret = copy_to_user(argp, &fix, sizeof(fix)) ? -EFAULT : 0; @@ -1326,6 +1328,8 @@ static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd, if (!lock_fb_info(info)) return -ENODEV; fix = info->fix; + if (info->flags & FBINFO_HIDE_SMEM_START) + fix.smem_start = 0; unlock_fb_info(info); return do_fscreeninfo_to_user(&fix, compat_ptr(arg)); } diff --git a/include/linux/fb.h b/include/linux/fb.h index fa8c6f9c9c3a..f42b09ca71f8 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -456,6 +456,13 @@ struct fb_tile_ops { * and host endianness. Drivers should not use this flag. */ #define FBINFO_BE_MATH 0x100000 +/* + * Hide smem_start in the FBIOGET_FSCREENINFO IOCTL. This is used by modern DRM + * drivers to stop userspace from trying to share buffers behind the kernel's + * back. Instead dma-buf based buffer sharing should be used. + */ +#define FBINFO_HIDE_SMEM_START 0x200000 + struct fb_info { atomic_t count; -- 2.18.0 ``` ## Re: \[PATCH 3/4\] fbdev: Add FBINFO\_HIDE\_SMEM\_START flag - From: Daniel Vetter \ - Date: 2018-09-03 16:48:54 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5370424.md ```text On Wed, Aug 22, 2018 at 10:54:04AM +0200, Daniel Vetter wrote: > DRM drivers really, really, really don't want random userspace to > share buffer behind it's back, bypassing the dma-buf buffer sharing > machanism. For that reason we've ruthlessly rejected any IOCTL > exposing the physical address of any graphics buffer. > > Unfortunately fbdev comes with that built-in. We could just set > smem_start to 0, but that means we'd have to hand-roll our own fb_mmap > implementation. For good reasons many drivers do that, but > smem_start/length is still super convenient. > > Hence instead just stop the leak in the ioctl, to keep fb mmap working > as-is. A second patch will set this flag for all drm drivers. > > Cc: Bartlomiej Zolnierkiewicz > Cc: Kees Cook > Cc: Daniel Vetter > Cc: > Signed-off-by: Daniel Vetter Any comments from the fbdev side on this? -Daniel > --- > drivers/video/fbdev/core/fbmem.c | 4 ++++ > include/linux/fb.h | 7 +++++++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index 609438d2465b..549d0f86fcf3 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1116,6 +1116,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, > if (!lock_fb_info(info)) > return -ENODEV; > fix = info->fix; > + if (info->flags & FBINFO_HIDE_SMEM_START) > + fix.smem_start = 0; > unlock_fb_info(info); > > ret = copy_to_user(argp, &fix, sizeof(fix)) ? -EFAULT : 0; > @@ -1326,6 +1328,8 @@ static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd, > if (!lock_fb_info(info)) > return -ENODEV; > fix = info->fix; > + if (info->flags & FBINFO_HIDE_SMEM_START) > + fix.smem_start = 0; > unlock_fb_info(info); > return do_fscreeninfo_to_user(&fix, compat_ptr(arg)); > } > diff --git a/include/linux/fb.h b/include/linux/fb.h > index fa8c6f9c9c3a..f42b09ca71f8 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -456,6 +456,13 @@ struct fb_tile_ops { > * and host endianness. Drivers should not use this flag. > */ > #define FBINFO_BE_MATH 0x100000 > +/* > + * Hide smem_start in the FBIOGET_FSCREENINFO IOCTL. This is used by modern DRM > + * drivers to stop userspace from trying to share buffers behind the kernel's > + * back. Instead dma-buf based buffer sharing should be used. > + */ > +#define FBINFO_HIDE_SMEM_START 0x200000 > + > > struct fb_info { > atomic_t count; > -- > 2.18.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ``` ## Re: \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - From: Bartlomiej Zolnierkiewicz \ - Date: 2018-09-10 12:48:53 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5376680.md ```text On 08/22/2018 10:54 AM, Daniel Vetter wrote: > This was only added for the drm's fbdev emulation support, so that it > would try harder to show the Oops. > > Unfortunately this never really worked reliably, and in practice ended > up pushing the real Oops off the screen due to plentyfull locking, > sleep-while-atomic and other issues. So we removed all that support > from the fbdev emulation a while back. Aside: We've also removed the > kgdb support, for similar reasons. > > Since it's such a small patch I figured I don't split this up into the > usual 3-phase removal. > > Cc: Ben Skeggs > Cc: Bartlomiej Zolnierkiewicz > Cc: Greg Kroah-Hartman > Cc: Hans de Goede > Cc: Daniel Vetter > Cc: Alexander Kapshuk > Cc: Kees Cook > Cc: Thierry Reding > Cc: David Lechner > Cc: > Cc: > Signed-off-by: Daniel Vetter Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ``` ## Re: \[PATCH 3/4\] fbdev: Add FBINFO\_HIDE\_SMEM\_START flag - From: Bartlomiej Zolnierkiewicz \ - Date: 2018-09-10 12:51:08 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5376682.md ```text On 08/22/2018 10:54 AM, Daniel Vetter wrote: > DRM drivers really, really, really don't want random userspace to > share buffer behind it's back, bypassing the dma-buf buffer sharing > machanism. For that reason we've ruthlessly rejected any IOCTL > exposing the physical address of any graphics buffer. > > Unfortunately fbdev comes with that built-in. We could just set > smem_start to 0, but that means we'd have to hand-roll our own fb_mmap > implementation. For good reasons many drivers do that, but > smem_start/length is still super convenient. > > Hence instead just stop the leak in the ioctl, to keep fb mmap working > as-is. A second patch will set this flag for all drm drivers. > > Cc: Bartlomiej Zolnierkiewicz > Cc: Kees Cook > Cc: Daniel Vetter > Cc: > Signed-off-by: Daniel Vetter Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ``` ## Re: \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - From: Daniel Vetter \ - Date: 2018-09-11 07:42:36 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5377451.md ```text On Mon, Sep 10, 2018 at 02:48:43PM +0200, Bartlomiej Zolnierkiewicz wrote: > > On 08/22/2018 10:54 AM, Daniel Vetter wrote: > > This was only added for the drm's fbdev emulation support, so that it > > would try harder to show the Oops. > > > > Unfortunately this never really worked reliably, and in practice ended > > up pushing the real Oops off the screen due to plentyfull locking, > > sleep-while-atomic and other issues. So we removed all that support > > from the fbdev emulation a while back. Aside: We've also removed the > > kgdb support, for similar reasons. > > > > Since it's such a small patch I figured I don't split this up into the > > usual 3-phase removal. > > > > Cc: Ben Skeggs > > Cc: Bartlomiej Zolnierkiewicz > > Cc: Greg Kroah-Hartman > > Cc: Hans de Goede > > Cc: Daniel Vetter > > Cc: Alexander Kapshuk > > Cc: Kees Cook > > Cc: Thierry Reding > > Cc: David Lechner > > Cc: > > Cc: > > Signed-off-by: Daniel Vetter > > Acked-by: Bartlomiej Zolnierkiewicz Ack for merging the entire series through drm-misc-next? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ``` ## Re: \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - From: Bartlomiej Zolnierkiewicz \ - Date: 2018-09-11 08:47:42 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5377513.md ```text On 09/11/2018 09:42 AM, Daniel Vetter wrote: > On Mon, Sep 10, 2018 at 02:48:43PM +0200, Bartlomiej Zolnierkiewicz wrote: >> >> On 08/22/2018 10:54 AM, Daniel Vetter wrote: >>> This was only added for the drm's fbdev emulation support, so that it >>> would try harder to show the Oops. >>> >>> Unfortunately this never really worked reliably, and in practice ended >>> up pushing the real Oops off the screen due to plentyfull locking, >>> sleep-while-atomic and other issues. So we removed all that support >>> from the fbdev emulation a while back. Aside: We've also removed the >>> kgdb support, for similar reasons. >>> >>> Since it's such a small patch I figured I don't split this up into the >>> usual 3-phase removal. >>> >>> Cc: Ben Skeggs >>> Cc: Bartlomiej Zolnierkiewicz >>> Cc: Greg Kroah-Hartman >>> Cc: Hans de Goede >>> Cc: Daniel Vetter >>> Cc: Alexander Kapshuk >>> Cc: Kees Cook >>> Cc: Thierry Reding >>> Cc: David Lechner >>> Cc: >>> Cc: >>> Signed-off-by: Daniel Vetter >> >> Acked-by: Bartlomiej Zolnierkiewicz > > Ack for merging the entire series through drm-misc-next? Yes, thanks for taking care of this. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ``` ## Re: \[Intel-gfx\] \[PATCH 1/4\] fbdev: Drop FBINFO\_CAN\_FORCE\_OUTPUT flag - From: Daniel Vetter \ - Date: 2018-09-11 12:12:23 - Message: https://ratatoskr.run/linux-fbdev/2018/09/5377685.md ```text On Tue, Sep 11, 2018 at 10:47:34AM +0200, Bartlomiej Zolnierkiewicz wrote: > On 09/11/2018 09:42 AM, Daniel Vetter wrote: > > On Mon, Sep 10, 2018 at 02:48:43PM +0200, Bartlomiej Zolnierkiewicz wrote: > >> > >> On 08/22/2018 10:54 AM, Daniel Vetter wrote: > >>> This was only added for the drm's fbdev emulation support, so that it > >>> would try harder to show the Oops. > >>> > >>> Unfortunately this never really worked reliably, and in practice ended > >>> up pushing the real Oops off the screen due to plentyfull locking, > >>> sleep-while-atomic and other issues. So we removed all that support > >>> from the fbdev emulation a while back. Aside: We've also removed the > >>> kgdb support, for similar reasons. > >>> > >>> Since it's such a small patch I figured I don't split this up into the > >>> usual 3-phase removal. > >>> > >>> Cc: Ben Skeggs > >>> Cc: Bartlomiej Zolnierkiewicz > >>> Cc: Greg Kroah-Hartman > >>> Cc: Hans de Goede > >>> Cc: Daniel Vetter > >>> Cc: Alexander Kapshuk > >>> Cc: Kees Cook > >>> Cc: Thierry Reding > >>> Cc: David Lechner > >>> Cc: > >>> Cc: > >>> Signed-off-by: Daniel Vetter > >> > >> Acked-by: Bartlomiej Zolnierkiewicz > > > > Ack for merging the entire series through drm-misc-next? > > Yes, thanks for taking care of this. Thanks, all merged into drm-misc-next for 4.20. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ```