On Thu, 19 Sep 2019, Greg KH wrote:
> On Thu, Sep 19, 2019 at 05:18:15PM +0800, Xiaoming Ni wrote:
> > Using kzalloc() to allocate memory in function con_init(), but not
> > checking the return value, there is a risk of null pointer references
> > oops.
> >
> > Signed-off-by: Xiaoming Ni <[email protected]>
>
> We keep having this be "reported" :(
>
> > ---
> > drivers/tty/vt/vt.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 34aa39d..db83e52 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -3357,15 +3357,33 @@ static int __init con_init(void)
> >
> > for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
> > vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data),
> > GFP_NOWAIT);
> > + if (unlikely(!vc)) {
> > + pr_warn("%s:failed to allocate memory for the %u vc\n",
> > + __func__, currcons);
> > + break;
> > + }
>
> At init, this really can not happen. Have you see it ever happen?
>
> > INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
> > tty_port_init(&vc->port);
> > visual_init(vc, currcons, 1);
> > vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
> > + if (unlikely(!vc->vc_screenbuf)) {
>
> Never use likely/unlikely unless you can actually measure the speed
> difference. For something like this, the compiler will always get it
> right without you having to do anything.
>
> And again, how can this fail? Have you seen it fail?
>
> > + pr_warn("%s:failed to allocate memory for the %u
> > vc_screenbuf\n",
> > + __func__, currcons);
> > + visual_deinit(vc);
> > + tty_port_destroy(&vc->port);
> > + kfree(vc);
> > + vc_cons[currcons].d = NULL;
> > + break;
> > + }
> > vc_init(vc, vc->vc_rows, vc->vc_cols,
> > currcons || !vc->vc_sw->con_save_screen);
> > }
> > currcons = fg_console = 0;
> > master_display_fg = vc = vc_cons[currcons].d;
> > + if (unlikely(!vc)) {
>
> Again, never use likely/unlikely unless you can measure it.
>
> thanks,
>
> greg k-h
Why does it use GFP_NOWAIT and not GFP_KERNEL? Is there some problem with
GFP_KERNEL during initialization?
Mikulas
