diff mbox

md/dm-table: tgt->type should be putted in dm_table_add_target()

Message ID 1477013732-4800-1-git-send-email-tang.junhui@zte.com.cn (mailing list archive)
State Accepted, archived
Delegated to: Mike Snitzer
Headers show

Commit Message

tang.junhui@zte.com.cn Oct. 21, 2016, 1:35 a.m. UTC
From: "tang.junhui" <tang.junhui@zte.com.cn>

tgt->type should be putted in dm_table_add_target()
when the target do not statisfy the needs of target type,
otherwise it would cause the module reference count
of this target type leakage.

Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
Cc: stable@vger.kernel.org
---
 drivers/md/dm-table.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Mike Snitzer Oct. 21, 2016, 2:16 a.m. UTC | #1
On Thu, Oct 20 2016 at  9:35pm -0400,
tang.junhui@zte.com.cn <tang.junhui@zte.com.cn> wrote:

> From: "tang.junhui" <tang.junhui@zte.com.cn>
> 
> tgt->type should be putted in dm_table_add_target()
> when the target do not statisfy the needs of target type,
> otherwise it would cause the module reference count
> of this target type leakage.
> 
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> Cc: stable@vger.kernel.org

This issue, missing dm_put_target_type, dates back to 2011.

See these 3 commits:
$ git log --oneline 3791e2fc0^..36a0456fb
36a0456 dm table: add immutable feature
cc6cbe1 dm table: add always writeable feature
3791e2f dm table: add singleton feature

So are you having problems with failed DM table loads (due to the errors
below) resulting in the inability to unload a DM module?

(BTW, you don't need to cc stable when initially proposing a patch for
stable.  The stable@ maintainers will automatically pull such stable@
changes in once they land in Linus' tree.)

Mike

> ---
>  drivers/md/dm-table.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 6554d91..4f56c38 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -698,30 +698,30 @@ int dm_table_add_target(struct dm_table *t, const char *type,
>  
>  	if (dm_target_needs_singleton(tgt->type)) {
>  		if (t->num_targets) {
> -			DMERR("%s: target type %s must appear alone in table",
> -			      dm_device_name(t->md), type);
> -			return -EINVAL;
> +			tgt->error = "target type must appear alone in table";
> +			r = -EINVAL;
> +			goto bad;
>  		}
>  		t->singleton = 1;
>  	}
>  
>  	if (dm_target_always_writeable(tgt->type) && !(t->mode & FMODE_WRITE)) {
> -		DMERR("%s: target type %s may not be included in read-only tables",
> -		      dm_device_name(t->md), type);
> -		return -EINVAL;
> +		tgt->error = "target type may not be included in read-only tables";
> +		r = -EINVAL;
> +		goto bad;
>  	}
>  
>  	if (t->immutable_target_type) {
>  		if (t->immutable_target_type != tgt->type) {
> -			DMERR("%s: immutable target type %s cannot be mixed with other target types",
> -			      dm_device_name(t->md), t->immutable_target_type->name);
> -			return -EINVAL;
> +			tgt->error = "immutable target type cannot be mixed with other target types";
> +			r = -EINVAL;
> +			goto bad;
>  		}
>  	} else if (dm_target_is_immutable(tgt->type)) {
>  		if (t->num_targets) {
> -			DMERR("%s: immutable target type %s cannot be mixed with other target types",
> -			      dm_device_name(t->md), tgt->type->name);
> -			return -EINVAL;
> +			tgt->error = "immutable target type cannot be mixed with other target types";
> +			r = -EINVAL;
> +			goto bad;
>  		}
>  		t->immutable_target_type = tgt->type;
>  	}
> -- 
> 2.8.1.windows.1
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
tang.junhui@zte.com.cn Oct. 21, 2016, 3:31 a.m. UTC | #2
Hello, Mike

Thanks for your quick respond. 
This problem causes failure in "rmmod dm-spp" command,
(dm-spp is a tagert type for our product)
since the reference count is not zero due to 
the failure of DM table loads.

I don't intend to cc stable, I just add the 
"Cc: stable@vger.kernel.org" after Signed-off-by in
commit message as Bart suggested it to me, then I send email
by "git send-email" command, and it cc to stable automatically,
I think maybe my git is not configured well, I'll research it 
later.

(BTW, I like your name, Mike, Thank you for your accompanying 
with us in our English class for so many years.) 

thanks,

Tang
 



发件人:         Mike Snitzer <snitzer@redhat.com>
收件人:         tang.junhui@zte.com.cn, 
抄送:   zhang.kai16@zte.com.cn, dm-devel@redhat.com, 
stable@vger.kernel.org, agk@redhat.com
日期:   2016/10/21 10:24
主题:   Re: [dm-devel] md/dm-table: tgt->type should be putted in 
dm_table_add_target()
发件人: dm-devel-bounces@redhat.com



On Thu, Oct 20 2016 at  9:35pm -0400,
tang.junhui@zte.com.cn <tang.junhui@zte.com.cn> wrote:

> From: "tang.junhui" <tang.junhui@zte.com.cn>

> 

> tgt->type should be putted in dm_table_add_target()

> when the target do not statisfy the needs of target type,

> otherwise it would cause the module reference count

> of this target type leakage.

> 

> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>

> Cc: stable@vger.kernel.org


This issue, missing dm_put_target_type, dates back to 2011.

See these 3 commits:
$ git log --oneline 3791e2fc0^..36a0456fb
36a0456 dm table: add immutable feature
cc6cbe1 dm table: add always writeable feature
3791e2f dm table: add singleton feature

So are you having problems with failed DM table loads (due to the errors
below) resulting in the inability to unload a DM module?

(BTW, you don't need to cc stable when initially proposing a patch for
stable.  The stable@ maintainers will automatically pull such stable@
changes in once they land in Linus' tree.)

Mike

> ---

>  drivers/md/dm-table.c | 24 ++++++++++++------------

>  1 file changed, 12 insertions(+), 12 deletions(-)

> 

> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c

> index 6554d91..4f56c38 100644

> --- a/drivers/md/dm-table.c

> +++ b/drivers/md/dm-table.c

> @@ -698,30 +698,30 @@ int dm_table_add_target(struct dm_table *t, const 

char *type,
> 

>                if (dm_target_needs_singleton(tgt->type)) {

>                                if (t->num_targets) {

> -                                              DMERR("%s: target type %s 

must appear alone in table",
> - dm_device_name(t->md), type);

> -                                              return -EINVAL;

> +                                              tgt->error = "target type 

must appear alone in table";
> +                                              r = -EINVAL;

> +                                              goto bad;

>                                }

>                                t->singleton = 1;

>                }

> 

>                if (dm_target_always_writeable(tgt->type) && !(t->mode & 

FMODE_WRITE)) {
> -                              DMERR("%s: target type %s may not be 

included in read-only tables",
> -                                    dm_device_name(t->md), type);

> -                              return -EINVAL;

> +                              tgt->error = "target type may not be 

included in read-only tables";
> +                              r = -EINVAL;

> +                              goto bad;

>                }

> 

>                if (t->immutable_target_type) {

>                                if (t->immutable_target_type != 

tgt->type) {
> -                                              DMERR("%s: immutable 

target type %s cannot be mixed with other target types",
> - dm_device_name(t->md), t->immutable_target_type->name);

> -                                              return -EINVAL;

> +                                              tgt->error = "immutable 

target type cannot be mixed with other target types";
> +                                              r = -EINVAL;

> +                                              goto bad;

>                                }

>                } else if (dm_target_is_immutable(tgt->type)) {

>                                if (t->num_targets) {

> -                                              DMERR("%s: immutable 

target type %s cannot be mixed with other target types",
> - dm_device_name(t->md), tgt->type->name);

> -                                              return -EINVAL;

> +                                              tgt->error = "immutable 

target type cannot be mixed with other target types";
> +                                              r = -EINVAL;

> +                                              goto bad;

>                                }

>                                t->immutable_target_type = tgt->type;

>                }

> -- 

> 2.8.1.windows.1

> 


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox

Patch

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 6554d91..4f56c38 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -698,30 +698,30 @@  int dm_table_add_target(struct dm_table *t, const char *type,
 
 	if (dm_target_needs_singleton(tgt->type)) {
 		if (t->num_targets) {
-			DMERR("%s: target type %s must appear alone in table",
-			      dm_device_name(t->md), type);
-			return -EINVAL;
+			tgt->error = "target type must appear alone in table";
+			r = -EINVAL;
+			goto bad;
 		}
 		t->singleton = 1;
 	}
 
 	if (dm_target_always_writeable(tgt->type) && !(t->mode & FMODE_WRITE)) {
-		DMERR("%s: target type %s may not be included in read-only tables",
-		      dm_device_name(t->md), type);
-		return -EINVAL;
+		tgt->error = "target type may not be included in read-only tables";
+		r = -EINVAL;
+		goto bad;
 	}
 
 	if (t->immutable_target_type) {
 		if (t->immutable_target_type != tgt->type) {
-			DMERR("%s: immutable target type %s cannot be mixed with other target types",
-			      dm_device_name(t->md), t->immutable_target_type->name);
-			return -EINVAL;
+			tgt->error = "immutable target type cannot be mixed with other target types";
+			r = -EINVAL;
+			goto bad;
 		}
 	} else if (dm_target_is_immutable(tgt->type)) {
 		if (t->num_targets) {
-			DMERR("%s: immutable target type %s cannot be mixed with other target types",
-			      dm_device_name(t->md), tgt->type->name);
-			return -EINVAL;
+			tgt->error = "immutable target type cannot be mixed with other target types";
+			r = -EINVAL;
+			goto bad;
 		}
 		t->immutable_target_type = tgt->type;
 	}