From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ajay Singh Date: Mon, 26 Mar 2018 15:47:48 +0000 Subject: Re: [PATCH] staging: wilc1000: check for kmalloc allocation failures Message-Id: <20180326210548.791f070a@ajaysk-VirtualBox> List-Id: References: <20180321191941.4126-1-colin.king@canonical.com> <1521662598.7999.33.camel@perches.com> In-Reply-To: <1521662598.7999.33.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Colin King Cc: Joe Perches , Aditya Shankar , Ganesh Krishna , Greg Kroah-Hartman , linux-wireless@vger.kernel.org, devel@driverdev.osuosl.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Thanks for submitting the patch. On Wed, 21 Mar 2018 13:03:18 -0700 Joe Perches wrote: > On Wed, 2018-03-21 at 19:19 +0000, Colin King wrote: > > From: Colin Ian King > > > > There are three kmalloc allocations that are not null checked which > > potentially could lead to null pointer dereference issues. Fix this > > by adding null pointer return checks. > > looks like all of these should be kmemdup or kstrdup > > > > > @@ -951,6 +955,10 @@ static s32 handle_connect(struct wilc_vif *vif, > > if (conn_attr->ssid) { > > hif_drv->usr_conn_req.ssid = kmalloc(conn_attr->ssid_len + 1, > > GFP_KERNEL); > > + if (!hif_drv->usr_conn_req.ssid) { > > + result = -ENOMEM; > > + goto error; > > + } > > memcpy(hif_drv->usr_conn_req.ssid, > > conn_attr->ssid, > > conn_attr->ssid_len); With this changes the Coverity reported warning is handled correctly. For further improvement to the patch, as Joe Perches suggested, its better to make use of kmemdup instead of kmalloc & memcpy. As kstrdup requires the source string to be NULL terminated('\0') and conn_attr->ssid might not contains the '\0' terminated string. So kmemdup with length of 'conn_attr->ssid_len' can be used instead. Please include the changes by using kmemdup() for all kmalloc/memcpy in this patch. Regards, Ajay