| 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: <201212101144.45967.vitas@nppfactor.kiev.ua>
Date: Mon, 10 Dec 2012 11:44:45 +0200
From: Vitalii Demianets <vitas@...factor.kiev.ua>
To: linux-kernel@...r.kernel.org
Cc: "Hans J. Koch" <hjk@...sjkoch.de>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
Cong Ding <dinggnu@...il.com>
Subject: [PATCH 1/2 v3] Fix memory freeing issues
1. uioinfo was kfreed based on the presence of pdev->dev.of_node, which was
obviously wrong and unrelated to the fact if uioinfo was allocated statically
or dynamically. This patch introduces new flag which clearly shows if uioinfo
was allocated dynamically and kfrees uioinfo based on that flag;
2. Fix: priv data was not freed in case platform_get_irq() failed. As it was
caused mainly by improper exit labels naming, labels were renamed too;
3. The case of uioinfo AND pdev->dev.of_node both NULL (not initialized
in platform data) was not treated properly.
Signed-off-by: Vitalii Demianets <vitas@...factor.kiev.ua>
---
v2: Constants naming style
v3: Comments: better wording in comment accompanying flags initialization &
removed obsoleted OF comment
---
drivers/uio/uio_pdrv_genirq.c | 47 ++++++++++++++++++++++++----------------
1 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index 42202cd..cc5233b 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -30,6 +30,11 @@
#define DRIVER_NAME "uio_pdrv_genirq"
+enum {
+ UIO_IRQ_DISABLED = 0,
+ UIO_INFO_ALLOCED = 1,
+};
+
struct uio_pdrv_genirq_platdata {
struct uio_info *uioinfo;
spinlock_t lock;
@@ -63,7 +68,7 @@ static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info *dev_info)
* remember the state so we can allow user space to enable it later.
*/
- if (!test_and_set_bit(0, &priv->flags))
+ if (!test_and_set_bit(UIO_IRQ_DISABLED, &priv->flags))
disable_irq_nosync(irq);
return IRQ_HANDLED;
@@ -83,10 +88,10 @@ static int uio_pdrv_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on)
spin_lock_irqsave(&priv->lock, flags);
if (irq_on) {
- if (test_and_clear_bit(0, &priv->flags))
+ if (test_and_clear_bit(UIO_IRQ_DISABLED, &priv->flags))
enable_irq(dev_info->irq);
} else {
- if (!test_and_set_bit(0, &priv->flags))
+ if (!test_and_set_bit(UIO_IRQ_DISABLED, &priv->flags))
disable_irq(dev_info->irq);
}
spin_unlock_irqrestore(&priv->lock, flags);
@@ -101,8 +106,9 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
struct uio_mem *uiomem;
int ret = -EINVAL;
int i;
+ bool uioinfo_alloced = false;
- if (!uioinfo) {
+ if (!uioinfo && pdev->dev.of_node) {
int irq;
/* alloc uioinfo for one device */
@@ -110,10 +116,11 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
if (!uioinfo) {
ret = -ENOMEM;
dev_err(&pdev->dev, "unable to kmalloc\n");
- goto bad2;
+ goto out;
}
uioinfo->name = pdev->dev.of_node->name;
uioinfo->version = "devicetree";
+ uioinfo_alloced = true;
/* Multiple IRQs are not supported */
irq = platform_get_irq(pdev, 0);
@@ -125,32 +132,35 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
if (!uioinfo || !uioinfo->name || !uioinfo->version) {
dev_err(&pdev->dev, "missing platform_data\n");
- goto bad0;
+ goto out_uioinfo;
}
if (uioinfo->handler || uioinfo->irqcontrol ||
uioinfo->irq_flags & IRQF_SHARED) {
dev_err(&pdev->dev, "interrupt configuration error\n");
- goto bad0;
+ goto out_uioinfo;
}
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
dev_err(&pdev->dev, "unable to kmalloc\n");
- goto bad0;
+ goto out_uioinfo;
}
priv->uioinfo = uioinfo;
spin_lock_init(&priv->lock);
- priv->flags = 0; /* interrupt is enabled to begin with */
+ /* Interrupt is enabled to begin with,
+ * that's why UIO_IRQ_DISABLED flag is not initially set.
+ */
+ priv->flags = uioinfo_alloced ? BIT_MASK(UIO_INFO_ALLOCED) : 0;
priv->pdev = pdev;
if (!uioinfo->irq) {
ret = platform_get_irq(pdev, 0);
if (ret < 0) {
dev_err(&pdev->dev, "failed to get IRQ\n");
- goto bad0;
+ goto out_priv;
}
uioinfo->irq = ret;
}
@@ -205,19 +215,19 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
ret = uio_register_device(&pdev->dev, priv->uioinfo);
if (ret) {
dev_err(&pdev->dev, "unable to register uio device\n");
- goto bad1;
+ goto out_pm;
}
platform_set_drvdata(pdev, priv);
return 0;
- bad1:
- kfree(priv);
+out_pm:
pm_runtime_disable(&pdev->dev);
- bad0:
- /* kfree uioinfo for OF */
- if (pdev->dev.of_node)
+out_priv:
+ kfree(priv);
+out_uioinfo:
+ if (uioinfo_alloced)
kfree(uioinfo);
- bad2:
+out:
return ret;
}
@@ -231,8 +241,7 @@ static int uio_pdrv_genirq_remove(struct platform_device *pdev)
priv->uioinfo->handler = NULL;
priv->uioinfo->irqcontrol = NULL;
- /* kfree uioinfo for OF */
- if (pdev->dev.of_node)
+ if (test_bit(UIO_INFO_ALLOCED, &priv->flags))
kfree(priv->uioinfo);
kfree(priv);
--
1.7.8.6
--
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