Re: [PATCH 4.19 66/78] fbcon: remove soft scrollback code

Linus Torvalds
2020-09-23T18:19:39Z

On Wed, Sep 23, 2020 at 1:44 AM Pavel Machek <pavel@denx.de> wrote:
>
> >
> > https://www.openwall.com/lists/oss-security/2020/09/15/2
>
> Thank you for the pointer!

Note that for me to be willing to take the softscollback code back, it
really would have to be more than just a revert and a trivial fix.

It would have to be more along the lines of "this is simplified and
verified". For example, at a minimum all the VT_RESIZE etc stuff would
have to clearly and obviously reset the soft-scrollback, so that we
simply can't have those kinds of issues.

If you look at that commit 50145474f6ef ("fbcon: remove soft
scrollback code") that removes the code, most of that code really
doesn't make much sense.

I dare anybody looking at that removed fbcon_redraw_softback()
function (or the different cases in fbcon_scrolldelta(), for that
matter) to claim they understand what it is doing. It's very odd code,
and it handles a lot of odd special cases.

So I wouldn't mind re-introducing the scrollback code, but the
reintroduced version needs to be simple and fairly _obvious_. It
would need to not have all those odd special cases, and it would need
to clearly not have any subtle issues with things like font or screen
resizing.

I'd also suggest that it not even try to handle the multiple virtual
console case.

And yes, some of it may involve also clarifying the code in the VT
code itself, so that the interactions with the cursor and VT switching
is more obvious. Maybe instead of trying to deal with a SW cursor when
scrollback is active, just hide the cursor entirely before doing
scrollback. And make change_console (and all the resizing) explicitly
reset scrollback etc.

So the reason the code got removed was that it was just very grotty
and hasn't really been maintained for over a decade.

In order to resurrect it we'd not just have to have a maintainer, the
whole "it's grotty and incomprehensible and has these nasty
interactions with random other things" would need to be addressed too.

Linus