On 2020/07/10 19:55, Greg Kroah-Hartman wrote:
>> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
>> index 48a8199f7845..8497e9206607 100644
>> --- a/drivers/tty/vt/vt.c
>> +++ b/drivers/tty/vt/vt.c
>> @@ -1126,7 +1126,7 @@ int vc_allocate(unsigned int currcons) /* return 0 on
>> success */
>> con_set_default_unimap(vc);
>>
>> vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL);
>> - if (!vc->vc_screenbuf)
>> + if (ZERO_OR_NULL_PTR(vc->vc_screenbuf))
>
> No, let's check this before we do kzalloc() please, that's just an odd
> way of doing an allocation we shouldn't have had to do.
OK. I can change to
+ if (vc->vc_screenbuf_size > KMALLOC_MAX_SIZE || !vc->vc_screenbuf_size)
+ goto err_free;
vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_KERNEL);
if (!vc->vc_screenbuf)
goto err_free;
like vc_do_resize() does. But I'm currently waiting for syzbot to test this
patch, for
I don't have an environment for reproducing this problem.
>
>> goto err_free;
>>
>> /* If no drivers have overridden us and the user didn't pass a
>> @@ -1212,7 +1212,7 @@ static int vc_do_resize(struct tty_struct *tty, struct
>> vc_data *vc,
>> if (new_cols == vc->vc_cols && new_rows == vc->vc_rows)
>> return 0;
>>
>> - if (new_screen_size > KMALLOC_MAX_SIZE)
>> + if (new_screen_size > KMALLOC_MAX_SIZE || !new_screen_size)
>> return -EINVAL;
>> newscreen = kzalloc(new_screen_size, GFP_USER);
>> if (!newscreen)
>> @@ -3393,6 +3393,7 @@ static int __init con_init(void)
>> INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
>> tty_port_init(&vc->port);
>> visual_init(vc, currcons, 1);
>> + /* Assuming vc->vc_screenbuf_size is sane here, for this is
>> __init code. */
>
> Shouldn't we also check this here, or before we get here, too?
This is an __init function. Can we somehow pass column=0 or row=0 ?
>
> Just checking the values and rejecting that as a valid screen size
> should be sufficient.
Hmm, where are we checking that column * row does not exceed UINT_MAX, given
that
"struct vc_data"->vc_{cols,rows,screenbuf_size} are "unsigned int" and we do
vc->vc_size_row = vc->vc_cols << 1;
vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row;
in visual_init() ? Don't we need to reject earlier?
