[PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[Brahmajit Das]

From: Mikulas Patocka <mpatocka@redhat.com>

From: Mikulas Patocka <mpatocka@redhat.com>

[ Upstream commit 66be40a14e496689e1f0add50118408e22c96169 ]

If some of the arguments "check_at_most_once", "ignore_zero_blocks",
"use_fec_from_device", "root_hash_sig_key_desc" were specified more than
once on the target line, a memory leak would happen.

This commit fixes the memory leak. It also fixes error handling in
verity_verify_sig_parse_opt_args.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Brahmajit Das <listout@listout.xyz>
---
 drivers/md/dm-verity-fec.c        |  4 ++++
 drivers/md/dm-verity-target.c     |  8 +++++++-
 drivers/md/dm-verity-verify-sig.c | 17 +++++++++++++----
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index 0c41949db784..631a887b487c 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -593,6 +593,10 @@ int verity_fec_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
 	(*argc)--;
 
 	if (!strcasecmp(arg_name, DM_VERITY_OPT_FEC_DEV)) {
+		if (v->fec->dev) {
+			ti->error = "FEC device already specified";
+			return -EINVAL;
+		}
 		r = dm_get_device(ti, arg_value, BLK_OPEN_READ, &v->fec->dev);
 		if (r) {
 			ti->error = "FEC device lookup failed";
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 3c427f18a04b..ed49bcbd224f 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -1120,6 +1120,9 @@ static int verity_alloc_most_once(struct dm_verity *v)
 {
 	struct dm_target *ti = v->ti;
 
+	if (v->validated_blocks)
+		return 0;
+
 	/* the bitset can only handle INT_MAX blocks */
 	if (v->data_blocks > INT_MAX) {
 		ti->error = "device too large to use check_at_most_once";
@@ -1143,6 +1146,9 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
 	struct dm_verity_io *io;
 	u8 *zero_data;
 
+	if (v->zero_digest)
+		return 0;
+
 	v->zero_digest = kmalloc(v->digest_size, GFP_KERNEL);
 
 	if (!v->zero_digest)
@@ -1577,7 +1583,7 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 			goto bad;
 	}
 
-	/* Root hash signature is  a optional parameter*/
+	/* Root hash signature is an optional parameter */
 	r = verity_verify_root_hash(root_hash_digest_to_validate,
 				    strlen(root_hash_digest_to_validate),
 				    verify_args.sig,
diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c
index a9e2c6c0a33c..d5261a0e4232 100644
--- a/drivers/md/dm-verity-verify-sig.c
+++ b/drivers/md/dm-verity-verify-sig.c
@@ -71,9 +71,14 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as,
 				     const char *arg_name)
 {
 	struct dm_target *ti = v->ti;
-	int ret = 0;
+	int ret;
 	const char *sig_key = NULL;
 
+	if (v->signature_key_desc) {
+		ti->error = DM_VERITY_VERIFY_ERR("root_hash_sig_key_desc already specified");
+		return -EINVAL;
+	}
+
 	if (!*argc) {
 		ti->error = DM_VERITY_VERIFY_ERR("Signature key not specified");
 		return -EINVAL;
@@ -83,14 +88,18 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as,
 	(*argc)--;
 
 	ret = verity_verify_get_sig_from_key(sig_key, sig_opts);
-	if (ret < 0)
+	if (ret < 0) {
 		ti->error = DM_VERITY_VERIFY_ERR("Invalid key specified");
+		return ret;
+	}
 
 	v->signature_key_desc = kstrdup(sig_key, GFP_KERNEL);
-	if (!v->signature_key_desc)
+	if (!v->signature_key_desc) {
+		ti->error = DM_VERITY_VERIFY_ERR("Could not allocate memory for signature key");
 		return -ENOMEM;
+	}
 
-	return ret;
+	return 0;
 }
 
 /*
-- 
2.49.0

Re: [PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[Brahmajit Das]

Greg, Shuah, Mikulas,
This is my first attempt at backporting an upstream patch (Part of Linux
kernel Bug Fixing Summer 2025). Please feel free to correct me, I'm open
to feedback.
I see I've added two From section, if that requires me to
resend a v2 of the patch, please let me know.

On 06.06.2025 01:41, Brahmajit Das wrote:
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> [ Upstream commit 66be40a14e496689e1f0add50118408e22c96169 ]
> 
> If some of the arguments "check_at_most_once", "ignore_zero_blocks",
> "use_fec_from_device", "root_hash_sig_key_desc" were specified more than
> once on the target line, a memory leak would happen.
> 
> This commit fixes the memory leak. It also fixes error handling in
> verity_verify_sig_parse_opt_args.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Brahmajit Das <listout@listout.xyz>
> ---
>  drivers/md/dm-verity-fec.c        |  4 ++++
>  drivers/md/dm-verity-target.c     |  8 +++++++-
>  drivers/md/dm-verity-verify-sig.c | 17 +++++++++++++----
>  3 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
> index 0c41949db784..631a887b487c 100644
> --- a/drivers/md/dm-verity-fec.c
> +++ b/drivers/md/dm-verity-fec.c
> @@ -593,6 +593,10 @@ int verity_fec_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
>  	(*argc)--;
>  
>  	if (!strcasecmp(arg_name, DM_VERITY_OPT_FEC_DEV)) {
> +		if (v->fec->dev) {
> +			ti->error = "FEC device already specified";
> +			return -EINVAL;
> +		}
>  		r = dm_get_device(ti, arg_value, BLK_OPEN_READ, &v->fec->dev);
>  		if (r) {
>  			ti->error = "FEC device lookup failed";
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 3c427f18a04b..ed49bcbd224f 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -1120,6 +1120,9 @@ static int verity_alloc_most_once(struct dm_verity *v)
>  {
>  	struct dm_target *ti = v->ti;
>  
> +	if (v->validated_blocks)
> +		return 0;
> +
>  	/* the bitset can only handle INT_MAX blocks */
>  	if (v->data_blocks > INT_MAX) {
>  		ti->error = "device too large to use check_at_most_once";
> @@ -1143,6 +1146,9 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
>  	struct dm_verity_io *io;
>  	u8 *zero_data;
>  
> +	if (v->zero_digest)
> +		return 0;
> +
>  	v->zero_digest = kmalloc(v->digest_size, GFP_KERNEL);
>  
>  	if (!v->zero_digest)
> @@ -1577,7 +1583,7 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  			goto bad;
>  	}
>  
> -	/* Root hash signature is  a optional parameter*/
> +	/* Root hash signature is an optional parameter */
>  	r = verity_verify_root_hash(root_hash_digest_to_validate,
>  				    strlen(root_hash_digest_to_validate),
>  				    verify_args.sig,
> diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c
> index a9e2c6c0a33c..d5261a0e4232 100644
> --- a/drivers/md/dm-verity-verify-sig.c
> +++ b/drivers/md/dm-verity-verify-sig.c
> @@ -71,9 +71,14 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as,
>  				     const char *arg_name)
>  {
>  	struct dm_target *ti = v->ti;
> -	int ret = 0;
> +	int ret;
>  	const char *sig_key = NULL;
>  
> +	if (v->signature_key_desc) {
> +		ti->error = DM_VERITY_VERIFY_ERR("root_hash_sig_key_desc already specified");
> +		return -EINVAL;
> +	}
> +
>  	if (!*argc) {
>  		ti->error = DM_VERITY_VERIFY_ERR("Signature key not specified");
>  		return -EINVAL;
> @@ -83,14 +88,18 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as,
>  	(*argc)--;
>  
>  	ret = verity_verify_get_sig_from_key(sig_key, sig_opts);
> -	if (ret < 0)
> +	if (ret < 0) {
>  		ti->error = DM_VERITY_VERIFY_ERR("Invalid key specified");
> +		return ret;
> +	}
>  
>  	v->signature_key_desc = kstrdup(sig_key, GFP_KERNEL);
> -	if (!v->signature_key_desc)
> +	if (!v->signature_key_desc) {
> +		ti->error = DM_VERITY_VERIFY_ERR("Could not allocate memory for signature key");
>  		return -ENOMEM;
> +	}
>  
> -	return ret;
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.49.0
> 
> 

-- 
Regards,
listout

Re: [PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[Greg KH]

On Fri, Jun 06, 2025 at 01:41:16AM +0530, Brahmajit Das wrote:
> From: Mikulas Patocka <mpatocka@redhat.com>
> 
> From: Mikulas Patocka <mpatocka@redhat.com>

Twice is not good :(

> 
> [ Upstream commit 66be40a14e496689e1f0add50118408e22c96169 ]
> 
> If some of the arguments "check_at_most_once", "ignore_zero_blocks",
> "use_fec_from_device", "root_hash_sig_key_desc" were specified more than
> once on the target line, a memory leak would happen.
> 
> This commit fixes the memory leak. It also fixes error handling in
> verity_verify_sig_parse_opt_args.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Brahmajit Das <listout@listout.xyz>
> ---
>  drivers/md/dm-verity-fec.c        |  4 ++++
>  drivers/md/dm-verity-target.c     |  8 +++++++-
>  drivers/md/dm-verity-verify-sig.c | 17 +++++++++++++----
>  3 files changed, 24 insertions(+), 5 deletions(-)

What kernel tree(s) is this for?

thanks,

greg k-h

Re: [PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Fri, Jun 06, 2025 at 02:00:52AM +0530, Brahmajit Das wrote:
> Greg, Shuah, Mikulas,
> This is my first attempt at backporting an upstream patch (Part of Linux
> kernel Bug Fixing Summer 2025). Please feel free to correct me, I'm open
> to feedback.
> I see I've added two From section, if that requires me to
> resend a v2 of the patch, please let me know.

Yes it does.

But you provide no information as to why this needs to be merged _now_
and not through the normal stable backport process that happens.  What
is different here that required you to send it to us?

And you forgot to mention what kernel branch this is for.

There is a stable kernel rules file that explains most of this.

thanks,

greg k-h

Re: [PATCH 1/1] dm-verity: fix a memory leak if some arguments are specified multiple times

[Brahmajit Das]

On 06.06.2025 08:04, Greg KH wrote:
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> http://daringfireball.net/2007/07/on_top
> 

Thank you, I'll keep this in mind from next time.
> On Fri, Jun 06, 2025 at 02:00:52AM +0530, Brahmajit Das wrote:
> > Greg, Shuah, Mikulas,
> > This is my first attempt at backporting an upstream patch (Part of Linux
> > kernel Bug Fixing Summer 2025). Please feel free to correct me, I'm open
> > to feedback.
> > I see I've added two From section, if that requires me to
> > resend a v2 of the patch, please let me know.
> 
> Yes it does.
> 
> But you provide no information as to why this needs to be merged _now_
> and not through the normal stable backport process that happens.  What
> is different here that required you to send it to us?
> 
> And you forgot to mention what kernel branch this is for.
> 
> There is a stable kernel rules file that explains most of this.
> 
I see, got it. Thanks a lot again. I'll also take a look at the stable
rules.
> thanks,
> 
> greg k-h
> 

-- 
Regards,
listout