[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <77748e07-8411-4ec1-98d3-496fe6ebda38@gmx.de>
Date: Sat, 2 Aug 2025 21:54:56 +0200
From: Helge Deller <deller@....de>
To: Jari Ruusu <jariruusu@...tonmail.com>, Jiri Slaby <jirislaby@...nel.org>
Cc: Yi Yang <yiyang13@...wei.com>, GONG Ruiqi <gongruiqi1@...wei.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sasha Levin <sashal@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Text mode VGA-console scrolling is broken in upstream & stable
trees
On 7/31/25 18:12, Jari Ruusu wrote:
> On Thursday, July 31st, 2025 at 10:22, Jiri Slaby <jirislaby@...nel.org> wrote:
>> At the time this was posted (privately and on security@), I commented:
>> =====
>> > --- a/drivers/video/console/vgacon.c
>> > +++ b/drivers/video/console/vgacon.c
>> > @@ -1168,7 +1168,7 @@ static bool vgacon_scroll(struct vc_data *c, unsigned int t, unsigned int b,
>> > c->vc_screenbuf_size - delta);
>> > c->vc_origin = vga_vram_end - c->vc_screenbuf_size;
>> > vga_rolled_over = 0;
>> > - } else
>> > + } else if (oldo - delta >= (unsigned long)c->vc_screenbuf)
>> > c->vc_origin -= delta;
>>
>> IMO you should also add:
>> else
>> c->vc_origin = c->vc_screenbuf;
>>
>> Or clamp 'delta' beforehand and don't add the 'if'.
>> =====
>> That did not happen, AFAICS. Care to test the above suggestion?
>
> My reading of the code in vgacon_scroll() is that it directly
> bit-bangs video-RAM and checks that scroll read/write accesses
> stay in range vga_vram_base...vga_vram_end-1.
>
> Checking that c->vc_origin end up being >= c->vc_screenbuf is
> wrong because in text mode it should be index to video-RAM.
Yes, correct.
> Quote from original "messed up" patch, fix for CVE-2025-38213:
>> By analyzing the vmcore, we found that vc->vc_origin was somehow placed
>> one line prior to vc->vc_screenbuf when vc was in KD_TEXT mode, and
>> further writings to /dev/vcs caused out-of-bounds reads (and writes
>> right after) in vcs_write_buf_noattr().
>>
>> Our further experiments show that in most cases, vc->vc_origin equals to
>> vga_vram_base when the console is in KD_TEXT mode, and it's around
>> vc->vc_screenbuf for the KD_GRAPHICS mode. But via triggerring a
>> TIOCL_SETVESABLANK ioctl beforehand, we can make vc->vc_origin be around
>> vc->vc_screenbuf while the console is in KD_TEXT mode, and then by
>> writing the special 'ESC M' control sequence to the tty certain times
>> (depends on the value of `vc->state.y - vc->vc_top`), we can eventually
>> move vc->vc_origin prior to vc->vc_screenbuf. Here's the PoC, tested on
>> QEMU:
>
> To me that sounds like the bug is in TIOCL_SETVESABLANK ioctl().
> It should not be changing c->vc_origin to point elsewhere
> other than video-RAM when the console is in text mode.
Yes, agreed.
> How about adding a check to begining of vgacon_scroll() that
> bails out early if c->vc_origin is not a valid index to video-RAM?
Possible, but that's more of a work-around.
It would be nice to find and fix the real problem.
I tried to reproduce the issue with the provided testcase from
the original patch, but so far I failed.
For now I've added a Revert of the original patch to the fbdev git tree,
so that VGA backward scrolling works again. This gives some time to
fix the KASAN report.
Helge
Powered by blists - more mailing lists