diff mbox series

[v2,1/2] xen/xenbus: reference count registered modules

Message ID 20191129134306.2738-2-pdurrant@amazon.com (mailing list archive)
State Superseded
Headers show
Series allow xen-blkback to be cleanly unloaded | expand

Commit Message

Paul Durrant Nov. 29, 2019, 1:43 p.m. UTC
To prevent a module being removed whilst attached to a frontend, and
hence xenbus calling into potentially invalid text, take a reference on
the module before calling the probe() method (dropping it if unsuccessful)
and drop the reference after returning from the remove() method.

NOTE: This allows the ad-hoc reference counting in xen-netback to be
      removed. This will be done in a subsequent patch.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>

v2:
 - New in v2
---
 drivers/xen/xenbus/xenbus_probe.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Jürgen Groß Nov. 29, 2019, 3:46 p.m. UTC | #1
On 29.11.19 14:43, Paul Durrant wrote:
> To prevent a module being removed whilst attached to a frontend, and
> hence xenbus calling into potentially invalid text, take a reference on
> the module before calling the probe() method (dropping it if unsuccessful)
> and drop the reference after returning from the remove() method.
> 
> NOTE: This allows the ad-hoc reference counting in xen-netback to be
>        removed. This will be done in a subsequent patch.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen
Jan Beulich Nov. 29, 2019, 4 p.m. UTC | #2
On 29.11.2019 14:43, Paul Durrant wrote:
> To prevent a module being removed whilst attached to a frontend, and

Why only frontend?

> hence xenbus calling into potentially invalid text, take a reference on
> the module before calling the probe() method (dropping it if unsuccessful)
> and drop the reference after returning from the remove() method.
> 
> NOTE: This allows the ad-hoc reference counting in xen-netback to be
>       removed. This will be done in a subsequent patch.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> 
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -232,9 +232,11 @@ int xenbus_dev_probe(struct device *_dev)
>  		return err;
>  	}
>  
> +	__module_get(drv->driver.owner);

I guess you really want try_module_get() and deal with it returning
false.

Jan
Paul Durrant Nov. 29, 2019, 4:08 p.m. UTC | #3
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 29 November 2019 16:01
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: xen-devel@lists.xenproject.org; linux-block@vger.kernel.org; linux-
> kernel@vger.kernel.org; Stefano Stabellini <sstabellini@kernel.org>; Boris
> Ostrovsky <boris.ostrovsky@oracle.com>; Juergen Gross <jgross@suse.com>
> Subject: Re: [PATCH v2 1/2] xen/xenbus: reference count registered modules
> 
> On 29.11.2019 14:43, Paul Durrant wrote:
> > To prevent a module being removed whilst attached to a frontend, and
> 
> Why only frontend?
> 

True. Originally this was only intended for backends, but I guess this should now be 'otherend' or some equivalent form of words.

> > hence xenbus calling into potentially invalid text, take a reference on
> > the module before calling the probe() method (dropping it if
> unsuccessful)
> > and drop the reference after returning from the remove() method.
> >
> > NOTE: This allows the ad-hoc reference counting in xen-netback to be
> >       removed. This will be done in a subsequent patch.
> >
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >
> > --- a/drivers/xen/xenbus/xenbus_probe.c
> > +++ b/drivers/xen/xenbus/xenbus_probe.c
> > @@ -232,9 +232,11 @@ int xenbus_dev_probe(struct device *_dev)
> >  		return err;
> >  	}
> >
> > +	__module_get(drv->driver.owner);
> 
> I guess you really want try_module_get() and deal with it returning
> false.
> 

Perhaps, yes.

  Paul

> Jan
diff mbox series

Patch

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 5b471889d723..5a4947690500 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -232,9 +232,11 @@  int xenbus_dev_probe(struct device *_dev)
 		return err;
 	}
 
+	__module_get(drv->driver.owner);
+
 	err = drv->probe(dev, id);
 	if (err)
-		goto fail;
+		goto fail_put;
 
 	err = watch_otherend(dev);
 	if (err) {
@@ -244,6 +246,8 @@  int xenbus_dev_probe(struct device *_dev)
 	}
 
 	return 0;
+fail_put:
+	module_put(drv->driver.owner);
 fail:
 	xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename);
 	xenbus_switch_state(dev, XenbusStateClosed);
@@ -263,6 +267,8 @@  int xenbus_dev_remove(struct device *_dev)
 	if (drv->remove)
 		drv->remove(dev);
 
+	module_put(drv->driver.owner);
+
 	free_otherend_details(dev);
 
 	xenbus_switch_state(dev, XenbusStateClosed);