All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] device-dax: fix cdev leak
@ 2017-02-23 19:22 ` Dan Williams
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-02-23 19:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jason Gunthorpe, linux-kernel, stable

If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
with a stale device number.

Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
Cc: <stable@vger.kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/dax.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index ed758b74ddf0..0f8008dd0b0c 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
 	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
 	rc = device_add(dev);
 	if (rc) {
+		cdev_del(&dax_dev->cdev);
 		put_device(dev);
 		return ERR_PTR(rc);
 	}

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH] device-dax: fix cdev leak
@ 2017-02-23 19:22 ` Dan Williams
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-02-23 19:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jason Gunthorpe, Logan Gunthorpe, linux-kernel, stable

If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
with a stale device number.

Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
Cc: <stable@vger.kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/dax.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index ed758b74ddf0..0f8008dd0b0c 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
 	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
 	rc = device_add(dev);
 	if (rc) {
+		cdev_del(&dax_dev->cdev);
 		put_device(dev);
 		return ERR_PTR(rc);
 	}

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH] device-dax: fix cdev leak
@ 2017-02-23 19:22 ` Dan Williams
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-02-23 19:22 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: Jason Gunthorpe, Logan Gunthorpe, linux-kernel, stable

If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
with a stale device number.

Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
Cc: <stable@vger.kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dax/dax.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index ed758b74ddf0..0f8008dd0b0c 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
 	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
 	rc = device_add(dev);
 	if (rc) {
+		cdev_del(&dax_dev->cdev);
 		put_device(dev);
 		return ERR_PTR(rc);
 	}

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] device-dax: fix cdev leak
  2017-02-23 19:22 ` Dan Williams
@ 2017-02-23 20:20     ` Jason Gunthorpe
  -1 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2017-02-23 20:20 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm-y27Ovi1pjclAfugRpC6u6w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	stable-u79uwXL29TY76Z2rM5mHXA

On Thu, Feb 23, 2017 at 11:22:03AM -0800, Dan Williams wrote:
> If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
> with a stale device number.
> 
> Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Cc: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
> Reported-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>  drivers/dax/dax.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
> index ed758b74ddf0..0f8008dd0b0c 100644
> +++ b/drivers/dax/dax.c
> @@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>  	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>  	rc = device_add(dev);
>  	if (rc) {
> +		cdev_del(&dax_dev->cdev);

This probably should call into unregister_dax_dev and just skip the
device_unregister part.

Once cdev_add returns it is possible for a mmap to have been created,
so cleanup after that point has to go through all the other
unregister_dax_dev steps.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] device-dax: fix cdev leak
@ 2017-02-23 20:20     ` Jason Gunthorpe
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2017-02-23 20:20 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-nvdimm, Logan Gunthorpe, linux-kernel, stable

On Thu, Feb 23, 2017 at 11:22:03AM -0800, Dan Williams wrote:
> If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
> with a stale device number.
> 
> Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
> Cc: <stable@vger.kernel.org>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>  drivers/dax/dax.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
> index ed758b74ddf0..0f8008dd0b0c 100644
> +++ b/drivers/dax/dax.c
> @@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>  	dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>  	rc = device_add(dev);
>  	if (rc) {
> +		cdev_del(&dax_dev->cdev);

This probably should call into unregister_dax_dev and just skip the
device_unregister part.

Once cdev_add returns it is possible for a mmap to have been created,
so cleanup after that point has to go through all the other
unregister_dax_dev steps.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] device-dax: fix cdev leak
  2017-02-23 20:20     ` Jason Gunthorpe
@ 2017-02-23 20:27         ` Dan Williams
  -1 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-02-23 20:27 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-nvdimm, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	stable-u79uwXL29TY76Z2rM5mHXA

On Thu, Feb 23, 2017 at 12:20 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Thu, Feb 23, 2017 at 11:22:03AM -0800, Dan Williams wrote:
>> If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
>> with a stale device number.
>>
>> Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> Cc: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
>> Reported-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>> Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>  drivers/dax/dax.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>> index ed758b74ddf0..0f8008dd0b0c 100644
>> +++ b/drivers/dax/dax.c
>> @@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>>       dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>>       rc = device_add(dev);
>>       if (rc) {
>> +             cdev_del(&dax_dev->cdev);
>
> This probably should call into unregister_dax_dev and just skip the
> device_unregister part.
>
> Once cdev_add returns it is possible for a mmap to have been created,
> so cleanup after that point has to go through all the other
> unregister_dax_dev steps.

Ah true. I was thinking the device node is not auto-created if the
device_add() fails, but there's theoretically nothing stopping
userspace from trying to hit this race with its own self-created
device-node.

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] device-dax: fix cdev leak
@ 2017-02-23 20:27         ` Dan Williams
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-02-23 20:27 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-nvdimm, Logan Gunthorpe, linux-kernel, stable

On Thu, Feb 23, 2017 at 12:20 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Thu, Feb 23, 2017 at 11:22:03AM -0800, Dan Williams wrote:
>> If device_add() fails, cleanup the cdev. Otherwise, we leak a kobj_map()
>> with a stale device number.
>>
>> Fixes: ba09c01d2fa8 ("dax: convert to the cdev api")
>> Cc: <stable@vger.kernel.org>
>> Cc: Logan Gunthorpe <logang@deltatee.com>
>> Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>  drivers/dax/dax.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>> index ed758b74ddf0..0f8008dd0b0c 100644
>> +++ b/drivers/dax/dax.c
>> @@ -724,6 +724,7 @@ struct dax_dev *devm_create_dax_dev(struct dax_region *dax_region,
>>       dev_set_name(dev, "dax%d.%d", dax_region->id, dax_dev->id);
>>       rc = device_add(dev);
>>       if (rc) {
>> +             cdev_del(&dax_dev->cdev);
>
> This probably should call into unregister_dax_dev and just skip the
> device_unregister part.
>
> Once cdev_add returns it is possible for a mmap to have been created,
> so cleanup after that point has to go through all the other
> unregister_dax_dev steps.

Ah true. I was thinking the device node is not auto-created if the
device_add() fails, but there's theoretically nothing stopping
userspace from trying to hit this race with its own self-created
device-node.

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-23 20:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-23 19:22 [PATCH] device-dax: fix cdev leak Dan Williams
2017-02-23 19:22 ` Dan Williams
2017-02-23 19:22 ` Dan Williams
     [not found] ` <148787772308.30127.18437190636864555810.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-02-23 20:20   ` Jason Gunthorpe
2017-02-23 20:20     ` Jason Gunthorpe
     [not found]     ` <20170223202030.GB26301-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-02-23 20:27       ` Dan Williams
2017-02-23 20:27         ` Dan Williams

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.