On 2020/07/16 19:00, Daniel Vetter wrote:

On Thu, Jul 16, 2020 at 12:29:00AM +0900, Tetsuo Handa wrote:

quoted

On 2020/07/16 0:12, Dan Carpenter wrote:

quoted

I've complained about integer overflows in fbdev for a long time...

What I'd like to see is something like the following maybe.  I don't
know how to get the vc_data in fbmem.c so it doesn't include your checks
for negative.

Yes. Like I said "Thus, I consider that we need more sanity/constraints checks." at
https://lore.kernel.org/lkml/b1e7dd6a-fc22-bba8-0abb-d3e779329bce@i-love.sakura.ne.jp/ (local) ,
we want basic checks. That's a task for fbdev people who should be familiar with
necessary constraints.

I think the worldwide supply of people who understand fbdev and willing to
work on it is roughly 0. So if someone wants to fix this mess properly
(which likely means adding tons of over/underflow checks at entry points,
since you're never going to catch the driver bugs, there's too many and
not enough people who care) they need to fix this themselves.

But I think we can enforce reasonable constraint which is much stricter than Dan's basic_checks()
(which used INT_MAX). For example, do we need to accept var->{xres,yres} >= 1048576, for
"32768 rows or cols" * "32 pixels per character" = 1045876 and vc_do_resize() accepts only
rows and cols < 32768 ?

Just to avoid confusion here.

quoted

Anyway, my two patches are small and low cost; can we apply these patches regardless
of basic checks?

Which two patches where?

[PATCH v3] vt: Reject zero-sized screen buffer size.
 from https://lkml.kernel.org/r/20200712111013.11881-1-penguin-kernel@I-love.SAKURA.ne.jp

[PATCH v2] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.
 from https://lkml.kernel.org/r/20200715015102.3814-1-penguin-kernel@I-love.SAKURA.ne.jp