lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130111173652.GB26287@phenom.dumpdata.com> Date: Fri, 11 Jan 2013 12:36:52 -0500 From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> To: Jan Beulich <JBeulich@...e.com> Cc: Olaf Hering <olaf@...fle.de>, Jens Axboe <axboe@...nel.dk>, xen-devel <xen-devel@...ts.xen.org>, linux-kernel@...r.kernel.org Subject: Re: [PATCH] xen-blkback: do not leak mode property On Thu, Dec 20, 2012 at 10:31:11AM +0000, Jan Beulich wrote: > "be->mode" is obtained from xenbus_read(), which does a kmalloc() for > the message body. The short string is never released, so do it along > with freeing "be" itself, and make sure the string isn't kept when > backend_changed() doesn't complete successfully (which made it > desirable to slightly re-structure that function, so that the error > cleanup can be done in one place). applied. > > Reported-by: Olaf Hering <olaf@...fle.de> > Signed-off-by: Jan Beulich <jbeulich@...e.com> > > --- > drivers/block/xen-blkback/xenbus.c | 49 ++++++++++++++++++------------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > --- 3.7/drivers/block/xen-blkback/xenbus.c > +++ 3.7-xen-blkback-mode-leak/drivers/block/xen-blkback/xenbus.c > @@ -366,6 +366,7 @@ static int xen_blkbk_remove(struct xenbu > be->blkif = NULL; > } > > + kfree(be->mode); > kfree(be); > dev_set_drvdata(&dev->dev, NULL); > return 0; > @@ -501,6 +502,7 @@ static void backend_changed(struct xenbu > = container_of(watch, struct backend_info, backend_watch); > struct xenbus_device *dev = be->dev; > int cdrom = 0; > + unsigned long handle; > char *device_type; > > DPRINTK(""); > @@ -520,10 +522,10 @@ static void backend_changed(struct xenbu > return; > } > > - if ((be->major || be->minor) && > - ((be->major != major) || (be->minor != minor))) { > - pr_warn(DRV_PFX "changing physical device (from %x:%x to %x:%x) not supported.\n", > - be->major, be->minor, major, minor); > + if (be->major | be->minor) { > + if (be->major != major || be->minor != minor) > + pr_warn(DRV_PFX "changing physical device (from %x:%x to %x:%x) not supported.\n", > + be->major, be->minor, major, minor); > return; > } > > @@ -541,36 +543,33 @@ static void backend_changed(struct xenbu > kfree(device_type); > } > > - if (be->major == 0 && be->minor == 0) { > - /* Front end dir is a number, which is used as the handle. */ > + /* Front end dir is a number, which is used as the handle. */ > + err = strict_strtoul(strrchr(dev->otherend, '/') + 1, 0, &handle); > + if (err) > + return; > > - char *p = strrchr(dev->otherend, '/') + 1; > - long handle; > - err = strict_strtoul(p, 0, &handle); > - if (err) > - return; > + be->major = major; > + be->minor = minor; > > - be->major = major; > - be->minor = minor; > - > - err = xen_vbd_create(be->blkif, handle, major, minor, > - (NULL == strchr(be->mode, 'w')), cdrom); > - if (err) { > - be->major = 0; > - be->minor = 0; > - xenbus_dev_fatal(dev, err, "creating vbd structure"); > - return; > - } > + err = xen_vbd_create(be->blkif, handle, major, minor, > + !strchr(be->mode, 'w'), cdrom); > > + if (err) > + xenbus_dev_fatal(dev, err, "creating vbd structure"); > + else { > err = xenvbd_sysfs_addif(dev); > if (err) { > xen_vbd_free(&be->blkif->vbd); > - be->major = 0; > - be->minor = 0; > xenbus_dev_fatal(dev, err, "creating sysfs entries"); > - return; > } > + } > > + if (err) { > + kfree(be->mode); > + be->mode = NULL; > + be->major = 0; > + be->minor = 0; > + } else { > /* We're potentially connected now */ > xen_update_blkif_status(be->blkif); > } > > > > xen-blkback: do not leak mode property > > "be->mode" is obtained from xenbus_read(), which does a kmalloc() for > the message body. The short string is never released, so do it along > with freeing "be" itself, and make sure the string isn't kept when > backend_changed() doesn't complete successfully (which made it > desirable to slightly re-structure that function, so that the error > cleanup can be done in one place). > > Reported-by: Olaf Hering <olaf@...fle.de> > Signed-off-by: Jan Beulich <jbeulich@...e.com> > > --- > drivers/block/xen-blkback/xenbus.c | 49 ++++++++++++++++++------------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > --- 3.7/drivers/block/xen-blkback/xenbus.c > +++ 3.7-xen-blkback-mode-leak/drivers/block/xen-blkback/xenbus.c > @@ -366,6 +366,7 @@ static int xen_blkbk_remove(struct xenbu > be->blkif = NULL; > } > > + kfree(be->mode); > kfree(be); > dev_set_drvdata(&dev->dev, NULL); > return 0; > @@ -501,6 +502,7 @@ static void backend_changed(struct xenbu > = container_of(watch, struct backend_info, backend_watch); > struct xenbus_device *dev = be->dev; > int cdrom = 0; > + unsigned long handle; > char *device_type; > > DPRINTK(""); > @@ -520,10 +522,10 @@ static void backend_changed(struct xenbu > return; > } > > - if ((be->major || be->minor) && > - ((be->major != major) || (be->minor != minor))) { > - pr_warn(DRV_PFX "changing physical device (from %x:%x to %x:%x) not supported.\n", > - be->major, be->minor, major, minor); > + if (be->major | be->minor) { > + if (be->major != major || be->minor != minor) > + pr_warn(DRV_PFX "changing physical device (from %x:%x to %x:%x) not supported.\n", > + be->major, be->minor, major, minor); > return; > } > > @@ -541,36 +543,33 @@ static void backend_changed(struct xenbu > kfree(device_type); > } > > - if (be->major == 0 && be->minor == 0) { > - /* Front end dir is a number, which is used as the handle. */ > + /* Front end dir is a number, which is used as the handle. */ > + err = strict_strtoul(strrchr(dev->otherend, '/') + 1, 0, &handle); > + if (err) > + return; > > - char *p = strrchr(dev->otherend, '/') + 1; > - long handle; > - err = strict_strtoul(p, 0, &handle); > - if (err) > - return; > + be->major = major; > + be->minor = minor; > > - be->major = major; > - be->minor = minor; > - > - err = xen_vbd_create(be->blkif, handle, major, minor, > - (NULL == strchr(be->mode, 'w')), cdrom); > - if (err) { > - be->major = 0; > - be->minor = 0; > - xenbus_dev_fatal(dev, err, "creating vbd structure"); > - return; > - } > + err = xen_vbd_create(be->blkif, handle, major, minor, > + !strchr(be->mode, 'w'), cdrom); > > + if (err) > + xenbus_dev_fatal(dev, err, "creating vbd structure"); > + else { > err = xenvbd_sysfs_addif(dev); > if (err) { > xen_vbd_free(&be->blkif->vbd); > - be->major = 0; > - be->minor = 0; > xenbus_dev_fatal(dev, err, "creating sysfs entries"); > - return; > } > + } > > + if (err) { > + kfree(be->mode); > + be->mode = NULL; > + be->major = 0; > + be->minor = 0; > + } else { > /* We're potentially connected now */ > xen_update_blkif_status(be->blkif); > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists