Re: [PATCH v2] fbdev: bitblit: bound-check glyph index in bit_putcs*

[Woody Suwalski <terraluna977@gmail.com>]

Woody Suwalski wrote:
> Vitaly Chikunov wrote:
>> Dear linux-fbdev, stable,
>>
>> On Mon, Oct 20, 2025 at 09:47:01PM +0800, Junjie Cao wrote:
>>> bit_putcs_aligned()/unaligned() derived the glyph pointer from the
>>> character value masked by 0xff/0x1ff, which may exceed the actual 
>>> font's
>>> glyph count and read past the end of the built-in font array.
>>> Clamp the index to the actual glyph count before computing the address.
>>>
>>> This fixes a global out-of-bounds read reported by syzbot.
>>>
>>> Reported-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=793cf822d213be1a74f2
>>> Tested-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
>> This commit is applied to v5.10.247 and causes a regression: when
>> switching VT with ctrl-alt-f2 the screen is blank or completely filled
>> with angle characters, then new text is not appearing (or not visible).
>>
>> This commit is found with git bisect from v5.10.246 to v5.10.247:
>>
>>    0998a6cb232674408a03e8561dc15aa266b2f53b is the first bad commit
>>    commit 0998a6cb232674408a03e8561dc15aa266b2f53b
>>    Author:     Junjie Cao <junjie.cao@intel.com>
>>    AuthorDate: 2025-10-20 21:47:01 +0800
>>    Commit:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>    CommitDate: 2025-12-07 06:08:07 +0900
>>
>>        fbdev: bitblit: bound-check glyph index in bit_putcs*
>>
>>        commit 18c4ef4e765a798b47980555ed665d78b71aeadf upstream.
>>
>>        bit_putcs_aligned()/unaligned() derived the glyph pointer from 
>> the
>>        character value masked by 0xff/0x1ff, which may exceed the 
>> actual font's
>>        glyph count and read past the end of the built-in font array.
>>        Clamp the index to the actual glyph count before computing the 
>> address.
>>
>>        This fixes a global out-of-bounds read reported by syzbot.
>>
>>        Reported-by: 
>> syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>        Closes: 
>> https://syzkaller.appspot.com/bug?extid=793cf822d213be1a74f2
>>        Tested-by: syzbot+793cf822d213be1a74f2@syzkaller.appspotmail.com
>>        Signed-off-by: Junjie Cao <junjie.cao@intel.com>
>>        Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>        Signed-off-by: Helge Deller <deller@gmx.de>
>>        Cc: stable@vger.kernel.org
>>        Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>>     drivers/video/fbdev/core/bitblit.c | 16 ++++++++++++----
>>     1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> The minimal reproducer in cli, after kernel is booted:
>>
>>    date >/dev/tty2; chvt 2
>>
>> and the date does not appear.
>>
>> Thanks,
>>
>> #regzbot introduced: 0998a6cb232674408a03e8561dc15aa266b2f53b
>>
>>> ---
>>> v1: 
>>> https://lore.kernel.org/linux-fbdev/5d237d1a-a528-4205-a4d8-71709134f1e1@suse.de/
>>> v1 -> v2:
>>>   - Fix indentation and add blank line after declarations with the 
>>> .pl helper
>>>   - No functional changes
>>>
>>>   drivers/video/fbdev/core/bitblit.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/bitblit.c 
>>> b/drivers/video/fbdev/core/bitblit.c
>>> index 9d2e59796c3e..085ffb44c51a 100644
>>> --- a/drivers/video/fbdev/core/bitblit.c
>>> +++ b/drivers/video/fbdev/core/bitblit.c
>>> @@ -79,12 +79,16 @@ static inline void bit_putcs_aligned(struct 
>>> vc_data *vc, struct fb_info *info,
>>>                        struct fb_image *image, u8 *buf, u8 *dst)
>>>   {
>>>       u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
>>> +    unsigned int charcnt = vc->vc_font.charcount;
>>>       u32 idx = vc->vc_font.width >> 3;
>>>       u8 *src;
>>>         while (cnt--) {
>>> -        src = vc->vc_font.data + (scr_readw(s++)&
>>> -                      charmask)*cellsize;
>>> +        u16 ch = scr_readw(s++) & charmask;
>>> +
>>> +        if (ch >= charcnt)
>>> +            ch = 0;
>>> +        src = vc->vc_font.data + (unsigned int)ch * cellsize;
>>>             if (attr) {
>>>               update_attr(buf, src, attr, vc);
>>> @@ -112,14 +116,18 @@ static inline void bit_putcs_unaligned(struct 
>>> vc_data *vc,
>>>                          u8 *dst)
>>>   {
>>>       u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
>>> +    unsigned int charcnt = vc->vc_font.charcount;
>>>       u32 shift_low = 0, mod = vc->vc_font.width % 8;
>>>       u32 shift_high = 8;
>>>       u32 idx = vc->vc_font.width >> 3;
>>>       u8 *src;
>>>         while (cnt--) {
>>> -        src = vc->vc_font.data + (scr_readw(s++)&
>>> -                      charmask)*cellsize;
>>> +        u16 ch = scr_readw(s++) & charmask;
>>> +
>>> +        if (ch >= charcnt)
>>> +            ch = 0;
>>> +        src = vc->vc_font.data + (unsigned int)ch * cellsize;
>>>             if (attr) {
>>>               update_attr(buf, src, attr, vc);
>>> -- 
>>> 2.48.1
>>>
> I have done the same bisecting work, too bad I did not notice Vitaly's 
> work earlier :-(
>
> There is a "cheap" workaround for systems before 5.11, (not addressing 
> the root issue but) working:
>
> diff --git a/drivers/video/fbdev/core/bitblit.c 
> b/drivers/video/fbdev/core/bitblit.c
> index 7c2fc9f..c5a1a9d 100644
> --- a/drivers/video/fbdev/core/bitblit.c
> +++ b/drivers/video/fbdev/core/bitblit.c
> @@ -86,7 +86,7 @@ static inline void bit_putcs_aligned(struct vc_data 
> *vc, struct fb_info *info,
>      while (cnt--) {
>          u16 ch = scr_readw(s++) & charmask;
>
> -        if (ch >= charcnt)
> +        if (charcnt && ch >= charcnt)
>              ch = 0;
>          src = vc->vc_font.data + (unsigned int)ch * cellsize;
>
> @@ -125,7 +125,7 @@ static inline void bit_putcs_unaligned(struct 
> vc_data *vc,
>      while (cnt--) {
>          u16 ch = scr_readw(s++) & charmask;
>
> -        if (ch >= charcnt)
> +        if (charcnt && ch >= charcnt)
>              ch = 0;
>          src = vc->vc_font.data + (unsigned int)ch * cellsize;
>
> I will try next to go full backport from 5.11 as Thorsten has suggested.
>
> However the bigger problem is that the fbdev patch has landed in the 
> 5.4.302 EOL, and essentially the 5.4 EOL kernel is now hanging broken :-(
>
> Thanks, Woody
>
I have tested the solution of backporting the series of patches from 
5.11, it seems to be working OK.
However for the soon-to-be-EOL 5.10 and already EOL'ed 5.4 I would 
suggest a simpler solution where we replace  most of the logic from 5.11 
with a hardcoded charcnt=256, if charcnt not set. This would take 
advantage of the bugfix from Junjie, and be a minimal change for the 
5.10 kernel (works on 5.4 as well)

--- a/drivers/video/fbdev/core/bitblit.c    2026-01-10 
16:28:37.438569812 -0500
+++ b/drivers/video/fbdev/core/bitblit.c    2026-01-10 
16:32:51.356236549 -0500
@@ -86,6 +86,8 @@ static inline void bit_putcs_aligned(str
      while (cnt--) {
          u16 ch = scr_readw(s++) & charmask;

+        if (charcnt == 0)
+            charcnt = 256;
          if (ch >= charcnt)
              ch = 0;
          src = vc->vc_font.data + (unsigned int)ch * cellsize;
@@ -125,6 +127,8 @@ static inline void bit_putcs_unaligned(s
      while (cnt--) {
          u16 ch = scr_readw(s++) & charmask;

+        if (charcnt == 0)
+            charcnt = 256;
          if (ch >= charcnt)
              ch = 0;
          src = vc->vc_font.data + (unsigned int)ch * cellsize;

Thanks, Woody