[PATCH v6 0/4] fbcon: Fixes for screen resolution changes

[PATCH v6 0/4] fbcon: Fixes for screen resolution changes

[Helge Deller]

This series fixes possible out-of-bound memory accesses when users trigger
screen resolutions changes with invalid input parameters, e.g. reconfigures
screen which is smaller than the current font size, or if the virtual screen
size is smaller than the physical screen size.

v6:
- final version
- added all missing: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

v5:
- swapped patch #2 and #3
- modified patch #3 to use registered_fb[con2fb_map[i]] instead of
  fbcon_info_from_console(). This is necessary that the patch can
  be cleanly pushed back to v5.4
- added cleanup patch #4 to fix up patch #3 again to use fbcon_info_from_console()
  for v5.19+

v4:
- merged former patch #2 and #3
- added more Reviewed-by
- new patch #2 needs more testing. Note added to comit message that
  another patch needs to be backport as well.

Helge Deller (4):
  fbcon: Disallow setting font bigger than screen size
  fbmem: Prevent invalid virtual screen sizes
  fbcon: Prevent that screen size is smaller than font size
  fbcon: Use fbcon_info_from_console() in fbcon_modechange_possible()

 drivers/video/fbdev/core/fbcon.c | 32 ++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c | 10 +++++++++-
 include/linux/fbcon.h            |  4 ++++
 3 files changed, 45 insertions(+), 1 deletion(-)

--
2.35.3

[PATCH v6 1/4] fbcon: Disallow setting font bigger than screen size

[Helge Deller]

Prevent that users set a font size which is bigger than the physical screen.
It's unlikely this may happen (because screens are usually much larger than the
fonts and each font char is limited to 32x32 pixels), but it may happen on
smaller screens/LCD displays.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org # v4.14+
---
 drivers/video/fbdev/core/fbcon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c4e91715ef00..e162d5e753e5 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2469,6 +2469,11 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 	if (charcount != 256 && charcount != 512)
 		return -EINVAL;

+	/* font bigger than screen resolution ? */
+	if (font->width  > FBCON_SWAP(info->var.rotate, info->var.xres, info->var.yres) ||
+	    font->height > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
+		return -EINVAL;
+
 	/* Make sure drawing engine can handle the font */
 	if (!(info->pixmap.blit_x & (1 << (font->width - 1))) ||
 	    !(info->pixmap.blit_y & (1 << (font->height - 1))))
--
2.35.3

[PATCH v6 2/4] fbmem: Prevent invalid virtual screen sizes

[Helge Deller]

Prevent that drivers or the user sets the virtual screen resolution
smaller than the physical screen resolution.  This is important, because
otherwise we may access memory outside of the graphics memory area.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org # v5.4+
---
 drivers/video/fbdev/core/fbmem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index afa2863670f3..5dfa4bbee642 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1006,6 +1006,12 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 	if (var->xres < 8 || var->yres < 8)
 		return -EINVAL;

+	/* make sure virtual resolution >= physical resolution */
+	if (var->xres_virtual < var->xres)
+		return -EINVAL;
+	if (var->yres_virtual < var->yres)
+		return -EINVAL;
+
 	/* Too huge resolution causes multiplication overflow. */
 	if (check_mul_overflow(var->xres, var->yres, &unused) ||
 	    check_mul_overflow(var->xres_virtual, var->yres_virtual, &unused))
--
2.35.3

Re: [PATCH v6 2/4] fbmem: Prevent invalid virtual screen sizes

[Geert Uytterhoeven]

Hi Helge,

On Sun, Jun 26, 2022 at 12:32 PM Helge Deller <deller@gmx.de> wrote:
> Prevent that drivers or the user sets the virtual screen resolution
> smaller than the physical screen resolution.  This is important, because
> otherwise we may access memory outside of the graphics memory area.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org # v5.4+

Thanks for your patch, which is now commit fe04405ce5de13a5 ("fbmem:
Prevent invalid virtual screen sizes") in fbdev/for-next.

> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1006,6 +1006,12 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>         if (var->xres < 8 || var->yres < 8)
>                 return -EINVAL;
>
> +       /* make sure virtual resolution >= physical resolution */
> +       if (var->xres_virtual < var->xres)
> +               return -EINVAL;
> +       if (var->yres_virtual < var->yres)
> +               return -EINVAL;

This breaks valid use cases (e.g. "fbset -xres <larger-value-than-before>") ,
as the FBIOPUT_VSCREENINFO rule is to round up invalid values,
if possible.

Individual drivers may not follow that rule, so you could indeed end up
with a virtual resolution here if such a driver fails to sanitize var.
So either you have to move this after the call to fbops.fb_check_var()
below, and/or change the code to enlarge virtual resolution to match
physical resolution (at the risk of introducing another regression
with an obscure driver?).

So I'd go for moving it below.  And perhaps add a WARN(), as this
is a driver bug?

> +
>         /* Too huge resolution causes multiplication overflow. */
>         if (check_mul_overflow(var->xres, var->yres, &unused) ||
>             check_mul_overflow(var->xres_virtual, var->yres_virtual, &unused))

Note that doing the multiplication overflow check before calling
fbops.fb_check_var() is fine, as too large values can never be
rounded up to a valid value.

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

Re: [PATCH v6 2/4] fbmem: Prevent invalid virtual screen sizes

[Helge Deller]

Hi Geert,

On 6/28/22 10:36, Geert Uytterhoeven wrote:
> On Sun, Jun 26, 2022 at 12:32 PM Helge Deller <deller@gmx.de> wrote:
>> Prevent that drivers or the user sets the virtual screen resolution
>> smaller than the physical screen resolution.  This is important, because
>> otherwise we may access memory outside of the graphics memory area.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: stable@vger.kernel.org # v5.4+
>
> Thanks for your patch, which is now commit fe04405ce5de13a5 ("fbmem:
> Prevent invalid virtual screen sizes") in fbdev/for-next.
>
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1006,6 +1006,12 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>>         if (var->xres < 8 || var->yres < 8)
>>                 return -EINVAL;
>>
>> +       /* make sure virtual resolution >= physical resolution */
>> +       if (var->xres_virtual < var->xres)
>> +               return -EINVAL;
>> +       if (var->yres_virtual < var->yres)
>> +               return -EINVAL;
>
> This breaks valid use cases (e.g. "fbset -xres <larger-value-than-before>") ,
> as the FBIOPUT_VSCREENINFO rule is to round up invalid values,
> if possible.

You are right, fbset doesn't change the virtual screen size (unless the value
was given), so indeed we need to round up vres values in FBIOPUT_VSCREENINFO.

> Individual drivers may not follow that rule, so you could indeed end up
> with a virtual resolution here if such a driver fails to sanitize var.
> So either you have to move this after the call to fbops.fb_check_var()
> below, and/or change the code to enlarge virtual resolution to match
> physical resolution (at the risk of introducing another regression
> with an obscure driver?).
>
> So I'd go for moving it below.  And perhaps add a WARN(), as this
> is a driver bug?

That was exactly how I implemented in the first round, but changed it
due to feedback.
I'll respin the patch.

Thanks for reviewing that series!

Helge
>>         /* Too huge resolution causes multiplication overflow. */
>>         if (check_mul_overflow(var->xres, var->yres, &unused) ||
>>             check_mul_overflow(var->xres_virtual, var->yres_virtual, &unused))
>
> Note that doing the multiplication overflow check before calling
> fbops.fb_check_var() is fine, as too large values can never be
> rounded up to a valid value.
>
> 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

[PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Helge Deller]

We need to prevent that users configure a screen size which is smaller than the
currently selected font size. Otherwise rendering chars on the screen will
access memory outside the graphics memory region.

This patch adds a new function fbcon_modechange_possible() which
implements this check and which later may be extended with other checks
if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
for a too small screen size.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org # v5.4+
---
 drivers/video/fbdev/core/fbcon.c | 27 +++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c |  4 +++-
 include/linux/fbcon.h            |  4 ++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index e162d5e753e5..69c7261ac334 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2736,6 +2736,33 @@ void fbcon_update_vcs(struct fb_info *info, bool all)
 }
 EXPORT_SYMBOL(fbcon_update_vcs);

+/* let fbcon check if it supports a new screen resolution */
+int fbcon_modechange_possible(struct fb_info *info, struct fb_var_screeninfo *var)
+{
+	struct fbcon_ops *ops = info->fbcon_par;
+	struct vc_data *vc;
+	int i;
+
+	WARN_CONSOLE_UNLOCKED();
+
+	if (!ops || ops->currcon < 0)
+		return -EINVAL;
+
+	/* prevent setting a screen size which is smaller than font size */
+	for (i = first_fb_vc; i <= last_fb_vc; i++) {
+		vc = vc_cons[i].d;
+		if (!vc || registered_fb[con2fb_map[i]] != info)
+			continue;
+
+		if (vc->vc_font.width  > FBCON_SWAP(var->rotate, var->xres, var->yres) ||
+		    vc->vc_font.height > FBCON_SWAP(var->rotate, var->yres, var->xres))
+			return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(fbcon_modechange_possible);
+
 int fbcon_mode_deleted(struct fb_info *info,
 		       struct fb_videomode *mode)
 {
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 5dfa4bbee642..b6e1d0f2b974 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			return -EFAULT;
 		console_lock();
 		lock_fb_info(info);
-		ret = fb_set_var(info, &var);
+		ret = fbcon_modechange_possible(info, &var);
+		if (!ret)
+			ret = fb_set_var(info, &var);
 		if (!ret)
 			fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
 		unlock_fb_info(info);
diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
index ff5596dd30f8..2382dec6d6ab 100644
--- a/include/linux/fbcon.h
+++ b/include/linux/fbcon.h
@@ -15,6 +15,8 @@ void fbcon_new_modelist(struct fb_info *info);
 void fbcon_get_requirement(struct fb_info *info,
 			   struct fb_blit_caps *caps);
 void fbcon_fb_blanked(struct fb_info *info, int blank);
+int  fbcon_modechange_possible(struct fb_info *info,
+			       struct fb_var_screeninfo *var);
 void fbcon_update_vcs(struct fb_info *info, bool all);
 void fbcon_remap_all(struct fb_info *info);
 int fbcon_set_con2fb_map_ioctl(void __user *argp);
@@ -33,6 +35,8 @@ static inline void fbcon_new_modelist(struct fb_info *info) {}
 static inline void fbcon_get_requirement(struct fb_info *info,
 					 struct fb_blit_caps *caps) {}
 static inline void fbcon_fb_blanked(struct fb_info *info, int blank) {}
+static inline int  fbcon_modechange_possible(struct fb_info *info,
+				struct fb_var_screeninfo *var) { return 0; }
 static inline void fbcon_update_vcs(struct fb_info *info, bool all) {}
 static inline void fbcon_remap_all(struct fb_info *info) {}
 static inline int fbcon_set_con2fb_map_ioctl(void __user *argp) { return 0; }
--
2.35.3

Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Geert Uytterhoeven]

Hi Helge,

On Sun, Jun 26, 2022 at 12:33 PM Helge Deller <deller@gmx.de> wrote:
> We need to prevent that users configure a screen size which is smaller than the
> currently selected font size. Otherwise rendering chars on the screen will
> access memory outside the graphics memory region.
>
> This patch adds a new function fbcon_modechange_possible() which
> implements this check and which later may be extended with other checks
> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
> for a too small screen size.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org # v5.4+

Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
Prevent that screen size is smaller than font size") in fbdev/for-next

> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c

> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>                         return -EFAULT;
>                 console_lock();
>                 lock_fb_info(info);
> -               ret = fb_set_var(info, &var);
> +               ret = fbcon_modechange_possible(info, &var);

Again, this should be done (if done at all) after the call to
fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.

What if the user just wants to display graphics, not text?
Can't the text console be disabled instead?

> +               if (!ret)
> +                       ret = fb_set_var(info, &var);
>                 if (!ret)
>                         fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
>                 unlock_fb_info(info);

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

Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Helge Deller]

On 6/28/22 10:39, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Sun, Jun 26, 2022 at 12:33 PM Helge Deller <deller@gmx.de> wrote:
>> We need to prevent that users configure a screen size which is smaller than the
>> currently selected font size. Otherwise rendering chars on the screen will
>> access memory outside the graphics memory region.
>>
>> This patch adds a new function fbcon_modechange_possible() which
>> implements this check and which later may be extended with other checks
>> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
>> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
>> for a too small screen size.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: stable@vger.kernel.org # v5.4+
>
> Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
> Prevent that screen size is smaller than font size") in fbdev/for-next
>
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>                         return -EFAULT;
>>                 console_lock();
>>                 lock_fb_info(info);
>> -               ret = fb_set_var(info, &var);
>> +               ret = fbcon_modechange_possible(info, &var);
>
> Again, this should be done (if done at all) after the call to
> fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.
>
> What if the user just wants to display graphics, not text?

Yes, I need to go back to an older version here too and check that
the test is only run on text consoles.
That check was dropped, due feedback that you could switch
back from graphics (e.g. X11) to text console at any time....so the
check for text-only is not correct.

> Can't the text console be disabled instead?

I think the solution is to return failure if switching back to text mode isn't possible if
fonts are bigger than the screen resolution. That will be another patch.

Thanks!

Helge


>
>> +               if (!ret)
>> +                       ret = fb_set_var(info, &var);
>>                 if (!ret)
>>                         fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
>>                 unlock_fb_info(info);
>
> 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

Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Geert Uytterhoeven]

Hi Helge,

On Tue, Jun 28, 2022 at 10:52 PM Helge Deller <deller@gmx.de> wrote:
> On 6/28/22 10:39, Geert Uytterhoeven wrote:
> > On Sun, Jun 26, 2022 at 12:33 PM Helge Deller <deller@gmx.de> wrote:
> >> We need to prevent that users configure a screen size which is smaller than the
> >> currently selected font size. Otherwise rendering chars on the screen will
> >> access memory outside the graphics memory region.
> >>
> >> This patch adds a new function fbcon_modechange_possible() which
> >> implements this check and which later may be extended with other checks
> >> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
> >> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
> >> for a too small screen size.
> >>
> >> Signed-off-by: Helge Deller <deller@gmx.de>
> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: stable@vger.kernel.org # v5.4+
> >
> > Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
> > Prevent that screen size is smaller than font size") in fbdev/for-next

> >> --- a/drivers/video/fbdev/core/fbmem.c
> >> +++ b/drivers/video/fbdev/core/fbmem.c
> >> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> >>                         return -EFAULT;
> >>                 console_lock();
> >>                 lock_fb_info(info);
> >> -               ret = fb_set_var(info, &var);
> >> +               ret = fbcon_modechange_possible(info, &var);
> >
> > Again, this should be done (if done at all) after the call to
> > fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.
> >
> > What if the user just wants to display graphics, not text?
>
> Yes, I need to go back to an older version here too and check that
> the test is only run on text consoles.
> That check was dropped, due feedback that you could switch
> back from graphics (e.g. X11) to text console at any time....so the
> check for text-only is not correct.
>
> > Can't the text console be disabled instead?
>
> I think the solution is to return failure if switching back to text mode isn't possible if
> fonts are bigger than the screen resolution. That will be another patch.

Isn't the font a per-VC setting? Hence can't you change resolution,
switch to a different VC, and run into this?

I think the only real solution is to set the number of text columns
and/or rows to zero, and make sure that is handled correctly.

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

Re: [PATCH v6 3/4] fbcon: Prevent that screen size is smaller than font size

[Helge Deller]

On 6/29/22 09:03, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Tue, Jun 28, 2022 at 10:52 PM Helge Deller <deller@gmx.de> wrote:
>> On 6/28/22 10:39, Geert Uytterhoeven wrote:
>>> On Sun, Jun 26, 2022 at 12:33 PM Helge Deller <deller@gmx.de> wrote:
>>>> We need to prevent that users configure a screen size which is smaller than the
>>>> currently selected font size. Otherwise rendering chars on the screen will
>>>> access memory outside the graphics memory region.
>>>>
>>>> This patch adds a new function fbcon_modechange_possible() which
>>>> implements this check and which later may be extended with other checks
>>>> if necessary.  The new function is called from the FBIOPUT_VSCREENINFO
>>>> ioctl handler in fbmem.c, which will return -EINVAL if userspace asked
>>>> for a too small screen size.
>>>>
>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: stable@vger.kernel.org # v5.4+
>>>
>>> Thanks for your patch, which is now commit f0b6a66d33ca6e7e ("fbcon:
>>> Prevent that screen size is smaller than font size") in fbdev/for-next
>
>>>> --- a/drivers/video/fbdev/core/fbmem.c
>>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>>> @@ -1112,7 +1112,9 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>>>                         return -EFAULT;
>>>>                 console_lock();
>>>>                 lock_fb_info(info);
>>>> -               ret = fb_set_var(info, &var);
>>>> +               ret = fbcon_modechange_possible(info, &var);
>>>
>>> Again, this should be done (if done at all) after the call to
>>> fb_ops.check_var(), as it breaks the FBIOPUT_VSCREENINFO rounding rule.
>>>
>>> What if the user just wants to display graphics, not text?
>>
>> Yes, I need to go back to an older version here too and check that
>> the test is only run on text consoles.
>> That check was dropped, due feedback that you could switch
>> back from graphics (e.g. X11) to text console at any time....so the
>> check for text-only is not correct.
>>
>>> Can't the text console be disabled instead?
>>
>> I think the solution is to return failure if switching back to text mode isn't possible if
>> fonts are bigger than the screen resolution. That will be another patch.
>
> Isn't the font a per-VC setting? Hence can't you change resolution,
> switch to a different VC, and run into this?
>
> I think the only real solution is to set the number of text columns
> and/or rows to zero, and make sure that is handled correctly.

I agree, there doesn't seem to be a simple solution.
On the other hand, such usecase seems very unlikely.
If you have a proposal for a pacth I'd welcome it.

Anyway, I've just sent out a new patch series. It does not include any patch
for this theoretical problem yet.

Helge

[PATCH v6 4/4] fbcon: Use fbcon_info_from_console() in fbcon_modechange_possible()

[Helge Deller]

Use the fbcon_info_from_console() wrapper which was added to kernel
v5.19 with commit 409d6c95f9c6 ("fbcon: Introduce wrapper for console->fb_info lookup").

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/video/fbdev/core/fbcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 69c7261ac334..2ab7515ac842 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2751,7 +2751,7 @@ int fbcon_modechange_possible(struct fb_info *info, struct fb_var_screeninfo *va
 	/* prevent setting a screen size which is smaller than font size */
 	for (i = first_fb_vc; i <= last_fb_vc; i++) {
 		vc = vc_cons[i].d;
-		if (!vc || registered_fb[con2fb_map[i]] != info)
+		if (!vc || fbcon_info_from_console(i) != info)
 			continue;

 		if (vc->vc_font.width  > FBCON_SWAP(var->rotate, var->xres, var->yres) ||
--
2.35.3

Re: [PATCH v6 0/4] fbcon: Fixes for screen resolution changes

[Helge Deller]

On 6/26/22 12:28, Helge Deller wrote:
> This series fixes possible out-of-bound memory accesses when users trigger
> screen resolutions changes with invalid input parameters, e.g. reconfigures
> screen which is smaller than the current font size, or if the virtual screen
> size is smaller than the physical screen size.

I've pull this series into the for-next branch of the fbdev git tree:
https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/log/?h=for-next

Helge