Re: [PATCH v3] vt: Reject zero-sized screen buffer size.

On Sun, Jul 12, 2020 at 08:10:12PM +0900, Tetsuo Handa wrote:

[...] @@ -1125,6 +1134,11 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */ if (!*vc->vc_uni_pagedir_loc) con_set_default_unimap(vc);

• err = -EINVAL;
• if (vc->vc_cols > VC_MAXCOL || vc->vc_rows > VC_MAXROW ||

•    vc->vc_screenbuf_size > KMALLOC_MAX_SIZE || !vc->vc_screenbuf_size)
  

• goto err_free;
  

• err = -ENOMEM; vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL); if (!vc->vc_screenbuf) goto err_free;

I realize this patch already landed, but I wanted to remind folks to use the check_*_overflow() helpers, which can make a lot of this kind of stuff easier to deal with.

For example, in this case, I think visual_init() could likely be changed to return success/failure and do all the sanity checking:

if (check_shl_overflow(vc->vc_cols, 1, &vc->vc_size_row) || check_mul_overflow(vc->vc_rows, vc->vc_size_row, &vc->vc_screenbuf_size)) return -EINVAL;