linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cifs: check kzalloc return
@ 2018-12-18 16:27 Nicholas Mc Guire
  2018-12-18 17:33 ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Mc Guire @ 2018-12-18 16:27 UTC (permalink / raw)
  To: Steve French; +Cc: linux-cifs, samba-technical, linux-kernel, Nicholas Mc Guire

kzalloc can return NULL so a check is needed. While there is a
check for ret_buf there is no check for the allocation of
ret_buf->crfid.fid - this check is thus added. Both call-sites
of tconInfoAlloc() check for NULL return of tconInfoAlloc()
so returning NULL on failure of kzalloc() here seems appropriate.
As the kzalloc() is the only thing here that can fail it is
moved to the beginning so as not to initialize other resources
on failure of kzalloc.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Fixes: 3d4ef9a15343 ("smb3: fix redundant opens on root")
---

Problem located with an experimental coccinelle script

While at it make checkpatch happy by using *ret_buf->crfid.fid
rather than struct cifs_fid.

Patch was compile tested with: x86_64_defconfig + CIFS=m
(with some unrelated smatch warnings and some pending cocci fixes)

Patch is against v4.20-rc7 (localversion-next is next-20181218)

 fs/cifs/misc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 8a41f4e..59efffe 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -113,6 +113,13 @@ tconInfoAlloc(void)
 	struct cifs_tcon *ret_buf;
 	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
 	if (ret_buf) {
+		ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid),
+					     GFP_KERNEL);
+		if (!ret_buf->crfid.fid) {
+			kfree(ret_buf);
+			return NULL;
+		}
+
 		atomic_inc(&tconInfoAllocCount);
 		ret_buf->tidStatus = CifsNew;
 		++ret_buf->tc_count;
@@ -120,8 +127,6 @@ tconInfoAlloc(void)
 		INIT_LIST_HEAD(&ret_buf->tcon_list);
 		spin_lock_init(&ret_buf->open_file_lock);
 		mutex_init(&ret_buf->crfid.fid_mutex);
-		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
-					     GFP_KERNEL);
 		spin_lock_init(&ret_buf->stat_lock);
 		atomic_set(&ret_buf->num_local_opens, 0);
 		atomic_set(&ret_buf->num_remote_opens, 0);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cifs: check kzalloc return
  2018-12-18 16:27 [PATCH] cifs: check kzalloc return Nicholas Mc Guire
@ 2018-12-18 17:33 ` Joe Perches
  2018-12-19  7:22   ` Nicholas Mc Guire
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2018-12-18 17:33 UTC (permalink / raw)
  To: Nicholas Mc Guire, Steve French; +Cc: linux-cifs, samba-technical, linux-kernel

On Tue, 2018-12-18 at 17:27 +0100, Nicholas Mc Guire wrote:
> kzalloc can return NULL so a check is needed. While there is a
> check for ret_buf there is no check for the allocation of
> ret_buf->crfid.fid - this check is thus added. Both call-sites
> of tconInfoAlloc() check for NULL return of tconInfoAlloc()
> so returning NULL on failure of kzalloc() here seems appropriate.
> As the kzalloc() is the only thing here that can fail it is
> moved to the beginning so as not to initialize other resources
> on failure of kzalloc.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> Fixes: 3d4ef9a15343 ("smb3: fix redundant opens on root")
> ---
> 
> Problem located with an experimental coccinelle script
> 
> While at it make checkpatch happy by using *ret_buf->crfid.fid
> rather than struct cifs_fid.
> 
> Patch was compile tested with: x86_64_defconfig + CIFS=m
> (with some unrelated smatch warnings and some pending cocci fixes)
> 
> Patch is against v4.20-rc7 (localversion-next is next-20181218)
[]
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
[]
> @@ -113,6 +113,13 @@ tconInfoAlloc(void)
>  	struct cifs_tcon *ret_buf;
>  	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
>  	if (ret_buf) {
> +		ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid),
> +					     GFP_KERNEL);
> +		if (!ret_buf->crfid.fid) {
> +			kfree(ret_buf);
> +			return NULL;
> +		}
> +
>  		atomic_inc(&tconInfoAllocCount);
>  		ret_buf->tidStatus = CifsNew;
>  		++ret_buf->tc_count;
> @@ -120,8 +127,6 @@ tconInfoAlloc(void)
>  		INIT_LIST_HEAD(&ret_buf->tcon_list);
>  		spin_lock_init(&ret_buf->open_file_lock);
>  		mutex_init(&ret_buf->crfid.fid_mutex);
> -		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
> -					     GFP_KERNEL);
>  		spin_lock_init(&ret_buf->stat_lock);
>  		atomic_set(&ret_buf->num_local_opens, 0);
>  		atomic_set(&ret_buf->num_remote_opens, 0);

Perhaps use a more common style by returning early on the
first possible failure too so the block can be unindented.

Maybe as a separate cleanup patch.
---
 fs/cifs/misc.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 113980dba4d8..bee203055b30 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -111,21 +111,27 @@ struct cifs_tcon *
 tconInfoAlloc(void)
 {
 	struct cifs_tcon *ret_buf;
-	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
-	if (ret_buf) {
-		atomic_inc(&tconInfoAllocCount);
-		ret_buf->tidStatus = CifsNew;
-		++ret_buf->tc_count;
-		INIT_LIST_HEAD(&ret_buf->openFileList);
-		INIT_LIST_HEAD(&ret_buf->tcon_list);
-		spin_lock_init(&ret_buf->open_file_lock);
-		mutex_init(&ret_buf->crfid.fid_mutex);
-		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
-					     GFP_KERNEL);
-		spin_lock_init(&ret_buf->stat_lock);
-		atomic_set(&ret_buf->num_local_opens, 0);
-		atomic_set(&ret_buf->num_remote_opens, 0);
+
+	ret_buf = kzalloc(sizeof(*ret_buf), GFP_KERNEL);
+	if (!ret_buf)
+		return NULL;
+	ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid), GFP_KERNEL);
+	if (!ret_buf->crfid.fid) {
+		kfree(ret_buf);
+		return NULL;
 	}
+
+	atomic_inc(&tconInfoAllocCount);
+	ret_buf->tidStatus = CifsNew;
+	++ret_buf->tc_count;
+	INIT_LIST_HEAD(&ret_buf->openFileList);
+	INIT_LIST_HEAD(&ret_buf->tcon_list);
+	spin_lock_init(&ret_buf->open_file_lock);
+	mutex_init(&ret_buf->crfid.fid_mutex);
+	spin_lock_init(&ret_buf->stat_lock);
+	atomic_set(&ret_buf->num_local_opens, 0);
+	atomic_set(&ret_buf->num_remote_opens, 0);
+
 	return ret_buf;
 }
 


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cifs: check kzalloc return
  2018-12-18 17:33 ` Joe Perches
@ 2018-12-19  7:22   ` Nicholas Mc Guire
  2018-12-19 10:57     ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Mc Guire @ 2018-12-19  7:22 UTC (permalink / raw)
  To: Joe Perches
  Cc: Nicholas Mc Guire, Steve French, linux-cifs, samba-technical,
	linux-kernel

On Tue, Dec 18, 2018 at 09:33:58AM -0800, Joe Perches wrote:
> On Tue, 2018-12-18 at 17:27 +0100, Nicholas Mc Guire wrote:
> > kzalloc can return NULL so a check is needed. While there is a
> > check for ret_buf there is no check for the allocation of
> > ret_buf->crfid.fid - this check is thus added. Both call-sites
> > of tconInfoAlloc() check for NULL return of tconInfoAlloc()
> > so returning NULL on failure of kzalloc() here seems appropriate.
> > As the kzalloc() is the only thing here that can fail it is
> > moved to the beginning so as not to initialize other resources
> > on failure of kzalloc.
> > 
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > Fixes: 3d4ef9a15343 ("smb3: fix redundant opens on root")
> > ---
> > 
> > Problem located with an experimental coccinelle script
> > 
> > While at it make checkpatch happy by using *ret_buf->crfid.fid
> > rather than struct cifs_fid.
> > 
> > Patch was compile tested with: x86_64_defconfig + CIFS=m
> > (with some unrelated smatch warnings and some pending cocci fixes)
> > 
> > Patch is against v4.20-rc7 (localversion-next is next-20181218)
> []
> > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> []
> > @@ -113,6 +113,13 @@ tconInfoAlloc(void)
> >  	struct cifs_tcon *ret_buf;
> >  	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
> >  	if (ret_buf) {
> > +		ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid),
> > +					     GFP_KERNEL);
> > +		if (!ret_buf->crfid.fid) {
> > +			kfree(ret_buf);
> > +			return NULL;
> > +		}
> > +
> >  		atomic_inc(&tconInfoAllocCount);
> >  		ret_buf->tidStatus = CifsNew;
> >  		++ret_buf->tc_count;
> > @@ -120,8 +127,6 @@ tconInfoAlloc(void)
> >  		INIT_LIST_HEAD(&ret_buf->tcon_list);
> >  		spin_lock_init(&ret_buf->open_file_lock);
> >  		mutex_init(&ret_buf->crfid.fid_mutex);
> > -		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
> > -					     GFP_KERNEL);
> >  		spin_lock_init(&ret_buf->stat_lock);
> >  		atomic_set(&ret_buf->num_local_opens, 0);
> >  		atomic_set(&ret_buf->num_remote_opens, 0);
> 
> Perhaps use a more common style by returning early on the
> first possible failure too so the block can be unindented.
>

yup the restructured cleanup would be the better way to go

rather than making this two patches I guess it would be the
best to just skip the intermediate kzalloc only cleanup -
atleast I see little value in that intermediat step - so
could you take care of the kzalloc() in your refactoring 
please ?

thx!
hofrat
 
> Maybe as a separate cleanup patch.
> ---
>  fs/cifs/misc.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index 113980dba4d8..bee203055b30 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -111,21 +111,27 @@ struct cifs_tcon *
>  tconInfoAlloc(void)
>  {
>  	struct cifs_tcon *ret_buf;
> -	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
> -	if (ret_buf) {
> -		atomic_inc(&tconInfoAllocCount);
> -		ret_buf->tidStatus = CifsNew;
> -		++ret_buf->tc_count;
> -		INIT_LIST_HEAD(&ret_buf->openFileList);
> -		INIT_LIST_HEAD(&ret_buf->tcon_list);
> -		spin_lock_init(&ret_buf->open_file_lock);
> -		mutex_init(&ret_buf->crfid.fid_mutex);
> -		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
> -					     GFP_KERNEL);
> -		spin_lock_init(&ret_buf->stat_lock);
> -		atomic_set(&ret_buf->num_local_opens, 0);
> -		atomic_set(&ret_buf->num_remote_opens, 0);
> +
> +	ret_buf = kzalloc(sizeof(*ret_buf), GFP_KERNEL);
> +	if (!ret_buf)
> +		return NULL;
> +	ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid), GFP_KERNEL);
> +	if (!ret_buf->crfid.fid) {
> +		kfree(ret_buf);
> +		return NULL;
>  	}
> +
> +	atomic_inc(&tconInfoAllocCount);
> +	ret_buf->tidStatus = CifsNew;
> +	++ret_buf->tc_count;
> +	INIT_LIST_HEAD(&ret_buf->openFileList);
> +	INIT_LIST_HEAD(&ret_buf->tcon_list);
> +	spin_lock_init(&ret_buf->open_file_lock);
> +	mutex_init(&ret_buf->crfid.fid_mutex);
> +	spin_lock_init(&ret_buf->stat_lock);
> +	atomic_set(&ret_buf->num_local_opens, 0);
> +	atomic_set(&ret_buf->num_remote_opens, 0);
> +
>  	return ret_buf;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cifs: check kzalloc return
  2018-12-19  7:22   ` Nicholas Mc Guire
@ 2018-12-19 10:57     ` Joe Perches
  2018-12-21  5:53       ` Steve French
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2018-12-19 10:57 UTC (permalink / raw)
  To: Nicholas Mc Guire
  Cc: Nicholas Mc Guire, Steve French, linux-cifs, samba-technical,
	linux-kernel

On Wed, 2018-12-19 at 08:22 +0100, Nicholas Mc Guire wrote:
> On Tue, Dec 18, 2018 at 09:33:58AM -0800, Joe Perches wrote:
> > On Tue, 2018-12-18 at 17:27 +0100, Nicholas Mc Guire wrote:
> > > kzalloc can return NULL so a check is needed. While there is a
> > > check for ret_buf there is no check for the allocation of
> > > ret_buf->crfid.fid - this check is thus added. Both call-sites
> > > of tconInfoAlloc() check for NULL return of tconInfoAlloc()
> > > so returning NULL on failure of kzalloc() here seems appropriate.
> > > As the kzalloc() is the only thing here that can fail it is
> > > moved to the beginning so as not to initialize other resources
> > > on failure of kzalloc.
> > > 
> > Perhaps use a more common style by returning early on the
> > first possible failure too so the block can be unindented.
[]
> > yup the restructured cleanup would be the better way to go
> 
> rather than making this two patches I guess it would be the
> best to just skip the intermediate kzalloc only cleanup -
> atleast I see little value in that intermediat step - so
> could you take care of the kzalloc() in your refactoring 
> please ?

I did that in the patch I proposed, which is trivial.
If you want to take it, please do.

If you want a sign-off:

Signed-off-by: Joe Perches <joe@perches.com>

> > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
[]
> > @@ -111,21 +111,27 @@ struct cifs_tcon *
> >  tconInfoAlloc(void)
> >  {
> >  	struct cifs_tcon *ret_buf;
> > -	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
> > -	if (ret_buf) {
> > -		atomic_inc(&tconInfoAllocCount);
> > -		ret_buf->tidStatus = CifsNew;
> > -		++ret_buf->tc_count;
> > -		INIT_LIST_HEAD(&ret_buf->openFileList);
> > -		INIT_LIST_HEAD(&ret_buf->tcon_list);
> > -		spin_lock_init(&ret_buf->open_file_lock);
> > -		mutex_init(&ret_buf->crfid.fid_mutex);
> > -		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
> > -					     GFP_KERNEL);
> > -		spin_lock_init(&ret_buf->stat_lock);
> > -		atomic_set(&ret_buf->num_local_opens, 0);
> > -		atomic_set(&ret_buf->num_remote_opens, 0);
> > +
> > +	ret_buf = kzalloc(sizeof(*ret_buf), GFP_KERNEL);
> > +	if (!ret_buf)
> > +		return NULL;
> > +	ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid), GFP_KERNEL);
> > +	if (!ret_buf->crfid.fid) {
> > +		kfree(ret_buf);
> > +		return NULL;
> >  	}
> > +
> > +	atomic_inc(&tconInfoAllocCount);
> > +	ret_buf->tidStatus = CifsNew;
> > +	++ret_buf->tc_count;
> > +	INIT_LIST_HEAD(&ret_buf->openFileList);
> > +	INIT_LIST_HEAD(&ret_buf->tcon_list);
> > +	spin_lock_init(&ret_buf->open_file_lock);
> > +	mutex_init(&ret_buf->crfid.fid_mutex);
> > +	spin_lock_init(&ret_buf->stat_lock);
> > +	atomic_set(&ret_buf->num_local_opens, 0);
> > +	atomic_set(&ret_buf->num_remote_opens, 0);
> > +
> >  	return ret_buf;
> >  }
> >  



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cifs: check kzalloc return
  2018-12-19 10:57     ` Joe Perches
@ 2018-12-21  5:53       ` Steve French
  0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2018-12-21  5:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Nicholas Mc Guire, Steve French, CIFS, samba-technical, LKML,
	Nicholas Mc Guire

[-- Attachment #1: Type: text/plain, Size: 3394 bytes --]

Updated the patch with Joe's signed off and Nicholas's reviewed-by and
pushed to cifs-2.6.git for-next

See attached.

On Wed, Dec 19, 2018 at 4:58 AM Joe Perches via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> On Wed, 2018-12-19 at 08:22 +0100, Nicholas Mc Guire wrote:
> > On Tue, Dec 18, 2018 at 09:33:58AM -0800, Joe Perches wrote:
> > > On Tue, 2018-12-18 at 17:27 +0100, Nicholas Mc Guire wrote:
> > > > kzalloc can return NULL so a check is needed. While there is a
> > > > check for ret_buf there is no check for the allocation of
> > > > ret_buf->crfid.fid - this check is thus added. Both call-sites
> > > > of tconInfoAlloc() check for NULL return of tconInfoAlloc()
> > > > so returning NULL on failure of kzalloc() here seems appropriate.
> > > > As the kzalloc() is the only thing here that can fail it is
> > > > moved to the beginning so as not to initialize other resources
> > > > on failure of kzalloc.
> > > >
> > > Perhaps use a more common style by returning early on the
> > > first possible failure too so the block can be unindented.
> []
> > > yup the restructured cleanup would be the better way to go
> >
> > rather than making this two patches I guess it would be the
> > best to just skip the intermediate kzalloc only cleanup -
> > atleast I see little value in that intermediat step - so
> > could you take care of the kzalloc() in your refactoring
> > please ?
>
> I did that in the patch I proposed, which is trivial.
> If you want to take it, please do.
>
> If you want a sign-off:
>
> Signed-off-by: Joe Perches <joe@perches.com>
>
> > > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> []
> > > @@ -111,21 +111,27 @@ struct cifs_tcon *
> > >  tconInfoAlloc(void)
> > >  {
> > >     struct cifs_tcon *ret_buf;
> > > -   ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
> > > -   if (ret_buf) {
> > > -           atomic_inc(&tconInfoAllocCount);
> > > -           ret_buf->tidStatus = CifsNew;
> > > -           ++ret_buf->tc_count;
> > > -           INIT_LIST_HEAD(&ret_buf->openFileList);
> > > -           INIT_LIST_HEAD(&ret_buf->tcon_list);
> > > -           spin_lock_init(&ret_buf->open_file_lock);
> > > -           mutex_init(&ret_buf->crfid.fid_mutex);
> > > -           ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
> > > -                                        GFP_KERNEL);
> > > -           spin_lock_init(&ret_buf->stat_lock);
> > > -           atomic_set(&ret_buf->num_local_opens, 0);
> > > -           atomic_set(&ret_buf->num_remote_opens, 0);
> > > +
> > > +   ret_buf = kzalloc(sizeof(*ret_buf), GFP_KERNEL);
> > > +   if (!ret_buf)
> > > +           return NULL;
> > > +   ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid), GFP_KERNEL);
> > > +   if (!ret_buf->crfid.fid) {
> > > +           kfree(ret_buf);
> > > +           return NULL;
> > >     }
> > > +
> > > +   atomic_inc(&tconInfoAllocCount);
> > > +   ret_buf->tidStatus = CifsNew;
> > > +   ++ret_buf->tc_count;
> > > +   INIT_LIST_HEAD(&ret_buf->openFileList);
> > > +   INIT_LIST_HEAD(&ret_buf->tcon_list);
> > > +   spin_lock_init(&ret_buf->open_file_lock);
> > > +   mutex_init(&ret_buf->crfid.fid_mutex);
> > > +   spin_lock_init(&ret_buf->stat_lock);
> > > +   atomic_set(&ret_buf->num_local_opens, 0);
> > > +   atomic_set(&ret_buf->num_remote_opens, 0);
> > > +
> > >     return ret_buf;
> > >  }
> > >
>
>
>


-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-check-kzalloc-return.patch --]
[-- Type: text/x-patch, Size: 2549 bytes --]

From 687f983f49d80037bee0125f431f8e0243c7ab77 Mon Sep 17 00:00:00 2001
From: Joe Perches <joe@perches.com>
Date: Thu, 20 Dec 2018 23:50:48 -0600
Subject: [PATCH] cifs: check kzalloc return

kzalloc can return NULL so an additional check is needed. While there
is a check for ret_buf there is no check for the allocation of
ret_buf->crfid.fid - this check is thus added. Both call-sites
of tconInfoAlloc() check for NULL return of tconInfoAlloc()
so returning NULL on failure of kzalloc() here seems appropriate.
As the kzalloc() is the only thing here that can fail it is
moved to the beginning so as not to initialize other resources
on failure of kzalloc.

Fixes: 3d4ef9a15343 ("smb3: fix redundant opens on root")
---

Problem located with an experimental coccinelle script

While at it make checkpatch happy by using *ret_buf->crfid.fid
rather than struct cifs_fid.

Signed-off-by: Joe Perches <joe@perches.com>
Reported-by: Nicholas Mc Guire <hofrat@osadl.org>
Reviewed-by: Nicholas Mc Guire <hofrat@osadl.org>
---
 fs/cifs/misc.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 113980dba4d8..bee203055b30 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -111,21 +111,27 @@ struct cifs_tcon *
 tconInfoAlloc(void)
 {
 	struct cifs_tcon *ret_buf;
-	ret_buf = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
-	if (ret_buf) {
-		atomic_inc(&tconInfoAllocCount);
-		ret_buf->tidStatus = CifsNew;
-		++ret_buf->tc_count;
-		INIT_LIST_HEAD(&ret_buf->openFileList);
-		INIT_LIST_HEAD(&ret_buf->tcon_list);
-		spin_lock_init(&ret_buf->open_file_lock);
-		mutex_init(&ret_buf->crfid.fid_mutex);
-		ret_buf->crfid.fid = kzalloc(sizeof(struct cifs_fid),
-					     GFP_KERNEL);
-		spin_lock_init(&ret_buf->stat_lock);
-		atomic_set(&ret_buf->num_local_opens, 0);
-		atomic_set(&ret_buf->num_remote_opens, 0);
+
+	ret_buf = kzalloc(sizeof(*ret_buf), GFP_KERNEL);
+	if (!ret_buf)
+		return NULL;
+	ret_buf->crfid.fid = kzalloc(sizeof(*ret_buf->crfid.fid), GFP_KERNEL);
+	if (!ret_buf->crfid.fid) {
+		kfree(ret_buf);
+		return NULL;
 	}
+
+	atomic_inc(&tconInfoAllocCount);
+	ret_buf->tidStatus = CifsNew;
+	++ret_buf->tc_count;
+	INIT_LIST_HEAD(&ret_buf->openFileList);
+	INIT_LIST_HEAD(&ret_buf->tcon_list);
+	spin_lock_init(&ret_buf->open_file_lock);
+	mutex_init(&ret_buf->crfid.fid_mutex);
+	spin_lock_init(&ret_buf->stat_lock);
+	atomic_set(&ret_buf->num_local_opens, 0);
+	atomic_set(&ret_buf->num_remote_opens, 0);
+
 	return ret_buf;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-12-21  5:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-18 16:27 [PATCH] cifs: check kzalloc return Nicholas Mc Guire
2018-12-18 17:33 ` Joe Perches
2018-12-19  7:22   ` Nicholas Mc Guire
2018-12-19 10:57     ` Joe Perches
2018-12-21  5:53       ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).