linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options
@ 2020-09-29 12:45 Zhihao Cheng
  2020-09-29 12:45 ` [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process Zhihao Cheng
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Zhihao Cheng @ 2020-09-29 12:45 UTC (permalink / raw)
  To: richard, s.hauer; +Cc: yi.zhang, linux-mtd, linux-kernel

Fix a memory leak after dumping authentication mount options in error
handling branch.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: <stable@vger.kernel.org>  # 4.20+
Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")
---
 fs/ubifs/super.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a2420c900275..6f85cd618766 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1141,6 +1141,18 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 	return 0;
 }
 
+/*
+ * ubifs_release_options - release mount parameters which have been dumped.
+ * @c: UBIFS file-system description object
+ */
+static void ubifs_release_options(struct ubifs_info *c)
+{
+	kfree(c->auth_key_name);
+	c->auth_key_name = NULL;
+	kfree(c->auth_hash_name);
+	c->auth_hash_name = NULL;
+}
+
 /**
  * destroy_journal - destroy journal data structures.
  * @c: UBIFS file-system description object
@@ -1650,8 +1662,7 @@ static void ubifs_umount(struct ubifs_info *c)
 	ubifs_lpt_free(c, 0);
 	ubifs_exit_authentication(c);
 
-	kfree(c->auth_key_name);
-	kfree(c->auth_hash_name);
+	ubifs_release_options(c);
 	kfree(c->cbuf);
 	kfree(c->rcvrd_mst_node);
 	kfree(c->mst_node);
@@ -2219,6 +2230,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 out_unlock:
 	mutex_unlock(&c->umount_mutex);
 out_close:
+	ubifs_release_options(c);
 	ubi_close_volume(c->ubi);
 out:
 	return err;
-- 
2.25.4


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

* [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process
  2020-09-29 12:45 [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Zhihao Cheng
@ 2020-09-29 12:45 ` Zhihao Cheng
  2020-09-30  6:58   ` Sascha Hauer
  2020-09-29 12:45 ` [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path Zhihao Cheng
  2020-09-30  7:09 ` [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Sascha Hauer
  2 siblings, 1 reply; 6+ messages in thread
From: Zhihao Cheng @ 2020-09-29 12:45 UTC (permalink / raw)
  To: richard, s.hauer; +Cc: yi.zhang, linux-mtd, linux-kernel

There is no need to dump authentication options while remounting,
because authentication initialization can only be doing once in
the first mount process. Dumping authentication mount options in
remount process may cause memory leak if UBIFS has already been
mounted with old authentication mount options.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: <stable@vger.kernel.org>  # 4.20+
Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")
---
 fs/ubifs/super.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 6f85cd618766..9796f5df2f7f 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1110,14 +1110,20 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 			break;
 		}
 		case Opt_auth_key:
-			c->auth_key_name = kstrdup(args[0].from, GFP_KERNEL);
-			if (!c->auth_key_name)
-				return -ENOMEM;
+			if (!is_remount) {
+				c->auth_key_name = kstrdup(args[0].from,
+								GFP_KERNEL);
+				if (!c->auth_key_name)
+					return -ENOMEM;
+			}
 			break;
 		case Opt_auth_hash_name:
-			c->auth_hash_name = kstrdup(args[0].from, GFP_KERNEL);
-			if (!c->auth_hash_name)
-				return -ENOMEM;
+			if (!is_remount) {
+				c->auth_hash_name = kstrdup(args[0].from,
+								GFP_KERNEL);
+				if (!c->auth_hash_name)
+					return -ENOMEM;
+			}
 			break;
 		case Opt_ignore:
 			break;
-- 
2.25.4


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

* [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path
  2020-09-29 12:45 [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Zhihao Cheng
  2020-09-29 12:45 ` [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process Zhihao Cheng
@ 2020-09-29 12:45 ` Zhihao Cheng
  2020-09-30  6:44   ` Sascha Hauer
  2020-09-30  7:09 ` [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Sascha Hauer
  2 siblings, 1 reply; 6+ messages in thread
From: Zhihao Cheng @ 2020-09-29 12:45 UTC (permalink / raw)
  To: richard, s.hauer; +Cc: yi.zhang, linux-mtd, linux-kernel

Release the authentication related resource in some error handling
branches in mount_ubifs().

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
Cc: <stable@vger.kernel.org>  # 4.20+
Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")
---
 fs/ubifs/super.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 9796f5df2f7f..732218ef6656 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1331,7 +1331,7 @@ static int mount_ubifs(struct ubifs_info *c)
 
 	err = ubifs_read_superblock(c);
 	if (err)
-		goto out_free;
+		goto out_auth;
 
 	c->probing = 0;
 
@@ -1343,18 +1343,18 @@ static int mount_ubifs(struct ubifs_info *c)
 		ubifs_err(c, "'compressor \"%s\" is not compiled in",
 			  ubifs_compr_name(c, c->default_compr));
 		err = -ENOTSUPP;
-		goto out_free;
+		goto out_auth;
 	}
 
 	err = init_constants_sb(c);
 	if (err)
-		goto out_free;
+		goto out_auth;
 
 	sz = ALIGN(c->max_idx_node_sz, c->min_io_size) * 2;
 	c->cbuf = kmalloc(sz, GFP_NOFS);
 	if (!c->cbuf) {
 		err = -ENOMEM;
-		goto out_free;
+		goto out_auth;
 	}
 
 	err = alloc_wbufs(c);
@@ -1629,6 +1629,8 @@ static int mount_ubifs(struct ubifs_info *c)
 	free_wbufs(c);
 out_cbuf:
 	kfree(c->cbuf);
+out_auth:
+	ubifs_exit_authentication(c);
 out_free:
 	kfree(c->write_reserve_buf);
 	kfree(c->bu.buf);
-- 
2.25.4


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

* Re: [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path
  2020-09-29 12:45 ` [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path Zhihao Cheng
@ 2020-09-30  6:44   ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2020-09-30  6:44 UTC (permalink / raw)
  To: Zhihao Cheng; +Cc: richard, yi.zhang, linux-mtd, linux-kernel

On Tue, Sep 29, 2020 at 08:45:31PM +0800, Zhihao Cheng wrote:
> Release the authentication related resource in some error handling
> branches in mount_ubifs().
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Cc: <stable@vger.kernel.org>  # 4.20+
> Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

> ---
>  fs/ubifs/super.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 9796f5df2f7f..732218ef6656 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1331,7 +1331,7 @@ static int mount_ubifs(struct ubifs_info *c)
>  
>  	err = ubifs_read_superblock(c);
>  	if (err)
> -		goto out_free;
> +		goto out_auth;
>  
>  	c->probing = 0;
>  
> @@ -1343,18 +1343,18 @@ static int mount_ubifs(struct ubifs_info *c)
>  		ubifs_err(c, "'compressor \"%s\" is not compiled in",
>  			  ubifs_compr_name(c, c->default_compr));
>  		err = -ENOTSUPP;
> -		goto out_free;
> +		goto out_auth;
>  	}
>  
>  	err = init_constants_sb(c);
>  	if (err)
> -		goto out_free;
> +		goto out_auth;
>  
>  	sz = ALIGN(c->max_idx_node_sz, c->min_io_size) * 2;
>  	c->cbuf = kmalloc(sz, GFP_NOFS);
>  	if (!c->cbuf) {
>  		err = -ENOMEM;
> -		goto out_free;
> +		goto out_auth;
>  	}
>  
>  	err = alloc_wbufs(c);
> @@ -1629,6 +1629,8 @@ static int mount_ubifs(struct ubifs_info *c)
>  	free_wbufs(c);
>  out_cbuf:
>  	kfree(c->cbuf);
> +out_auth:
> +	ubifs_exit_authentication(c);
>  out_free:
>  	kfree(c->write_reserve_buf);
>  	kfree(c->bu.buf);
> -- 
> 2.25.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process
  2020-09-29 12:45 ` [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process Zhihao Cheng
@ 2020-09-30  6:58   ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2020-09-30  6:58 UTC (permalink / raw)
  To: Zhihao Cheng; +Cc: richard, yi.zhang, linux-mtd, linux-kernel

On Tue, Sep 29, 2020 at 08:45:30PM +0800, Zhihao Cheng wrote:
> There is no need to dump authentication options while remounting,
> because authentication initialization can only be doing once in
> the first mount process. Dumping authentication mount options in
> remount process may cause memory leak if UBIFS has already been
> mounted with old authentication mount options.
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Cc: <stable@vger.kernel.org>  # 4.20+
> Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  fs/ubifs/super.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 6f85cd618766..9796f5df2f7f 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1110,14 +1110,20 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>  			break;
>  		}
>  		case Opt_auth_key:
> -			c->auth_key_name = kstrdup(args[0].from, GFP_KERNEL);
> -			if (!c->auth_key_name)
> -				return -ENOMEM;
> +			if (!is_remount) {
> +				c->auth_key_name = kstrdup(args[0].from,
> +								GFP_KERNEL);
> +				if (!c->auth_key_name)
> +					return -ENOMEM;
> +			}
>  			break;
>  		case Opt_auth_hash_name:
> -			c->auth_hash_name = kstrdup(args[0].from, GFP_KERNEL);
> -			if (!c->auth_hash_name)
> -				return -ENOMEM;
> +			if (!is_remount) {
> +				c->auth_hash_name = kstrdup(args[0].from,
> +								GFP_KERNEL);
> +				if (!c->auth_hash_name)
> +					return -ENOMEM;
> +			}
>  			break;
>  		case Opt_ignore:
>  			break;
> -- 
> 2.25.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options
  2020-09-29 12:45 [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Zhihao Cheng
  2020-09-29 12:45 ` [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process Zhihao Cheng
  2020-09-29 12:45 ` [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path Zhihao Cheng
@ 2020-09-30  7:09 ` Sascha Hauer
  2 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2020-09-30  7:09 UTC (permalink / raw)
  To: Zhihao Cheng; +Cc: richard, yi.zhang, linux-mtd, linux-kernel

On Tue, Sep 29, 2020 at 08:45:29PM +0800, Zhihao Cheng wrote:
> Fix a memory leak after dumping authentication mount options in error
> handling branch.
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Cc: <stable@vger.kernel.org>  # 4.20+
> Fixes: d8a22773a12c6d7 ("ubifs: Enable authentication support")

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

I wonder if patches like in this series should really go to stable. There's
always the risk of regressions, and there's not much to win in fixing
such low probability, low frequency memory holes.

Sascha

> ---
>  fs/ubifs/super.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index a2420c900275..6f85cd618766 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1141,6 +1141,18 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>  	return 0;
>  }
>  
> +/*
> + * ubifs_release_options - release mount parameters which have been dumped.
> + * @c: UBIFS file-system description object
> + */
> +static void ubifs_release_options(struct ubifs_info *c)
> +{
> +	kfree(c->auth_key_name);
> +	c->auth_key_name = NULL;
> +	kfree(c->auth_hash_name);
> +	c->auth_hash_name = NULL;
> +}
> +
>  /**
>   * destroy_journal - destroy journal data structures.
>   * @c: UBIFS file-system description object
> @@ -1650,8 +1662,7 @@ static void ubifs_umount(struct ubifs_info *c)
>  	ubifs_lpt_free(c, 0);
>  	ubifs_exit_authentication(c);
>  
> -	kfree(c->auth_key_name);
> -	kfree(c->auth_hash_name);
> +	ubifs_release_options(c);
>  	kfree(c->cbuf);
>  	kfree(c->rcvrd_mst_node);
>  	kfree(c->mst_node);
> @@ -2219,6 +2230,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  out_unlock:
>  	mutex_unlock(&c->umount_mutex);
>  out_close:
> +	ubifs_release_options(c);
>  	ubi_close_volume(c->ubi);
>  out:
>  	return err;
> -- 
> 2.25.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

end of thread, other threads:[~2020-09-30  7:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 12:45 [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Zhihao Cheng
2020-09-29 12:45 ` [PATCH 2/3] ubifs: Don't parse authentication mount options in remount process Zhihao Cheng
2020-09-30  6:58   ` Sascha Hauer
2020-09-29 12:45 ` [PATCH 3/3] ubifs: mount_ubifs: Release authentication resource in error handling path Zhihao Cheng
2020-09-30  6:44   ` Sascha Hauer
2020-09-30  7:09 ` [PATCH 1/3] ubifs: Fix a memleak after dumping authentication mount options Sascha Hauer

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).