Re: Possible bug in drivers/tty/vt/vt.c

Re: Possible bug in drivers/tty/vt/vt.c

[Anthony Canino]

Hi all,

I hope this is the right place to ask about a potential bug in the TTY
that I may have found in the TTY layer in the linux kernel. I have
failed a bug report
(https://bugzilla.kernel.org/show_bug.cgi?id=208293) but wanted to
email the list for the TTY layer directly. In summary, in the con_init
function of drivers/tty/vt/vt.c, I think this code is possibly buggy
is kzalloc fails to allocate:

  3391   for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
  3392     vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data),
GFP_NOWAIT);
  3393     INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
  3394     tty_port_init(&vc->port);
  3395     visual_init(vc, currcons, 1);
  3396     vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
  3397     vc_init(vc, vc->vc_rows, vc->vc_cols,
  3398       currcons || !vc->vc_sw->con_save_screen);
  3399   }
  3400   currcons = fg_console = 0;
  3401   master_display_fg = vc = vc_cons[currcons].d;
  3402   set_origin(vc);

If kzalloc returns null on 3396, I think during set_origin(vc) it is
possible vc_screenbuf will be dereferenced. I'd be happy to discuss
further if needed.

Thanks,
Anthony


On Tue, Jun 23, 2020 at 5:24 AM Anthony Canino
<anthony.canino1@gmail.com> wrote:
>
> Hi all,
>
> I hope this is the right place to ask about a potential bug in the TTY that I may have found in the TTY layer in the linux kernel. I have failed a bug report (https://bugzilla.kernel.org/show_bug.cgi?id=208293) but wanted to email the list for the TTY layer directly. In summary, in the con_init function of drivers/tty/vt/vt.c, I think this code is possibly buggy is kzalloc fails to allocate:
>
>   3391   for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
>   3392     vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data), GFP_NOWAIT);
>   3393     INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
>   3394     tty_port_init(&vc->port);
>   3395     visual_init(vc, currcons, 1);
>   3396     vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
>   3397     vc_init(vc, vc->vc_rows, vc->vc_cols,
>   3398       currcons || !vc->vc_sw->con_save_screen);
>   3399   }
>   3400   currcons = fg_console = 0;
>   3401   master_display_fg = vc = vc_cons[currcons].d;
>   3402   set_origin(vc);
>
> If kzalloc returns null on 3396, I think during set_origin(vc) it is possible vc_screenbuf will be dereferenced. I'd be happy to discuss further if needed.
>
> Thanks,
> Anthony



-- 
Anthony

Re: Possible bug in drivers/tty/vt/vt.c

[Greg KH]

On Tue, Jun 23, 2020 at 05:27:33AM -0400, Anthony Canino wrote:
> Hi all,
> 
> I hope this is the right place to ask about a potential bug in the TTY
> that I may have found in the TTY layer in the linux kernel. I have
> failed a bug report
> (https://bugzilla.kernel.org/show_bug.cgi?id=208293) but wanted to
> email the list for the TTY layer directly. In summary, in the con_init
> function of drivers/tty/vt/vt.c, I think this code is possibly buggy
> is kzalloc fails to allocate:
> 
>   3391   for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
>   3392     vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data),
> GFP_NOWAIT);
>   3393     INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
>   3394     tty_port_init(&vc->port);
>   3395     visual_init(vc, currcons, 1);
>   3396     vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
>   3397     vc_init(vc, vc->vc_rows, vc->vc_cols,
>   3398       currcons || !vc->vc_sw->con_save_screen);
>   3399   }
>   3400   currcons = fg_console = 0;
>   3401   master_display_fg = vc = vc_cons[currcons].d;
>   3402   set_origin(vc);
> 
> If kzalloc returns null on 3396, I think during set_origin(vc) it is
> possible vc_screenbuf will be dereferenced. I'd be happy to discuss
> further if needed.

Yes, horrible and bad things will happen if kzalloc fails at that point
in time.

Luckily, it is impossible for that to happen, so we really do not need
to worry about it at all.  This comes up every other year or so, and the
gyrations that people have gone through to try to fix this up, for
something that is impossible to ever hit, always end up breaking the
codebase or doing other horrible things.

In short, don't worry about it, unless you can show me how that can ever
happen in a normal (i.e. not instrumented) system?

thanks,

greg k-h

Re: Possible bug in drivers/tty/vt/vt.c

[Anthony Canino]

Hi Greg,

Thanks for the reply--what you said makes sense. If we find a case
where this happens in a normal execution, we will file another bug.

Thanks!

Anthony

On Tue, Jun 23, 2020 at 7:51 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 23, 2020 at 05:27:33AM -0400, Anthony Canino wrote:
> > Hi all,
> >
> > I hope this is the right place to ask about a potential bug in the TTY
> > that I may have found in the TTY layer in the linux kernel. I have
> > failed a bug report
> > (https://bugzilla.kernel.org/show_bug.cgi?id=208293) but wanted to
> > email the list for the TTY layer directly. In summary, in the con_init
> > function of drivers/tty/vt/vt.c, I think this code is possibly buggy
> > is kzalloc fails to allocate:
> >
> >   3391   for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
> >   3392     vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data),
> > GFP_NOWAIT);
> >   3393     INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
> >   3394     tty_port_init(&vc->port);
> >   3395     visual_init(vc, currcons, 1);
> >   3396     vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
> >   3397     vc_init(vc, vc->vc_rows, vc->vc_cols,
> >   3398       currcons || !vc->vc_sw->con_save_screen);
> >   3399   }
> >   3400   currcons = fg_console = 0;
> >   3401   master_display_fg = vc = vc_cons[currcons].d;
> >   3402   set_origin(vc);
> >
> > If kzalloc returns null on 3396, I think during set_origin(vc) it is
> > possible vc_screenbuf will be dereferenced. I'd be happy to discuss
> > further if needed.
>
> Yes, horrible and bad things will happen if kzalloc fails at that point
> in time.
>
> Luckily, it is impossible for that to happen, so we really do not need
> to worry about it at all.  This comes up every other year or so, and the
> gyrations that people have gone through to try to fix this up, for
> something that is impossible to ever hit, always end up breaking the
> codebase or doing other horrible things.
>
> In short, don't worry about it, unless you can show me how that can ever
> happen in a normal (i.e. not instrumented) system?
>
> thanks,
>
> greg k-h



-- 
Anthony

Re: Possible bug in drivers/tty/vt/vt.c

[Greg KH]

On Thu, Jun 25, 2020 at 10:44:27AM -0400, Anthony Canino wrote:
> Hi Greg,
> 
> Thanks for the reply--what you said makes sense. If we find a case
> where this happens in a normal execution, we will file another bug.

Just email please, that's the best way to work on things, bugzilla is a
black hole...