>>> On 05.12.12 at 11:01, Olaf Hering <o...@aepfle.de> wrote:
> backend_changed might be called multiple times, which will leak
> be->mode. free the previous value before storing the current mode value.
As said before - this is one possible route to take. But did you
consider at all the alternative of preventing the function from
getting called more than once for a given device? As also said
before, I think that would have other bad effects, and hence
should be preferred (and would likely also result in a smaller
patch).
And _if_ this here is the route to go, I'd clearly see this and your
earlier patch to be folded into just one (dealing with both leaks
in one go).
Jan
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
>
> !! Not compile tested !!
>
> drivers/block/xen-blkback/xenbus.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/xenbus.c
> b/drivers/block/xen-blkback/xenbus.c
> index a6585a4..8650b19 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -502,7 +502,7 @@ static void backend_changed(struct xenbus_watch *watch,
> = container_of(watch, struct backend_info, backend_watch);
> struct xenbus_device *dev = be->dev;
> int cdrom = 0;
> - char *device_type;
> + char *mode, *device_type;
>
> DPRINTK("");
>
> @@ -528,13 +528,15 @@ static void backend_changed(struct xenbus_watch *watch,
> return;
> }
>
> - be->mode = xenbus_read(XBT_NIL, dev->nodename, "mode", NULL);
> - if (IS_ERR(be->mode)) {
> - err = PTR_ERR(be->mode);
> + mode = xenbus_read(XBT_NIL, dev->nodename, "mode", NULL);
> + kfree(be->mode);
> + if (IS_ERR(mode)) {
> + err = PTR_ERR(mode);
> be->mode = NULL;
> xenbus_dev_fatal(dev, err, "reading mode");
> return;
> }
> + be->mode = mode;
>
> device_type = xenbus_read(XBT_NIL, dev->otherend, "device-type", NULL);
> if (!IS_ERR(device_type)) {
> @@ -555,7 +557,7 @@ static void backend_changed(struct xenbus_watch *watch,
> be->minor = minor;
>
> err = xen_vbd_create(be->blkif, handle, major, minor,
> - (NULL == strchr(be->mode, 'w')), cdrom);
> + (NULL == strchr(mode, 'w')), cdrom);
> if (err) {
> be->major = 0;
> be->minor = 0;
> --
> 1.8.0.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
