Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 20 Sep 2019 08:04:54 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Nixiaoming <nixiaoming@huawei.com>
Cc:     "penberg@cs.helsinki.fi" <penberg@cs.helsinki.fi>,
        "jslaby@suse.com" <jslaby@suse.com>,
        "nico@fluxnic.net" <nico@fluxnic.net>,
        "textshell@uchuujin.de" <textshell@uchuujin.de>,
        "sam@ravnborg.org" <sam@ravnborg.org>,
        "daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
        "mpatocka@redhat.com" <mpatocka@redhat.com>,
        "ghalat@redhat.com" <ghalat@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Yangyingliang <yangyingliang@huawei.com>,
        yuehaibing <yuehaibing@huawei.com>,
        Zengweilin <zengweilin@huawei.com>
Subject: Re: [PATCH] tty:vt: Add check the return value of kzalloc to avoid
 oops
Message-ID: <20190920060454.GB473496@kroah.com>
References: <1568884695-56789-1-git-send-email-nixiaoming@huawei.com>
 <20190919092933.GA2684163@kroah.com>
 <E490CD805F7529488761C40FD9D26EF12AE5F4FA@dggemm507-mbx.china.huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <E490CD805F7529488761C40FD9D26EF12AE5F4FA@dggemm507-mbx.china.huawei.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 20, 2019 at 02:29:49AM +0000, Nixiaoming wrote:
> On 2019/9/19 17:30, 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 <nixiaoming@huawei.com>
> >
> > 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?
> 
> I did not actually observe the null pointer here.
> I am confused when I see the code allocated here without check the return value.
> Small memory allocation failures are difficult to occur during system initialization
> But is it not safe enough if the code is not judged?
> Also if the memory allocation failure is not allowed here, is it better to add the __GFP_NOFAIL flags?

See my response to Nicolas, but yes, that would be a good way to handle
this.

thanks,

greg k-h