linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Wu <lekensteyn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Martin Mokrejs
	<mmokrejs-08dBlVkRsZWoiTQjSSYKZesEoJ4y9sgM@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Peter Wu <lekensteyn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: [PATCH] i2c: i801: fix memleak on probe error
Date: Mon, 23 Dec 2013 10:39:38 +0100	[thread overview]
Message-ID: <1387791578-1372-1-git-send-email-lekensteyn@gmail.com> (raw)
In-Reply-To: <20130123174204.00463f98-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>

The driver-specific data for i801 was only set for the device on
success, that led to a memory leak on error paths (for instance, when
there is a resource conflict with ACPI). (The driver core clears the
driver data (if set) if the probe routine fails).

Fix it by setting the driver data right after successful memory
allocation, before reaching any error paths.

References: http://lkml.org/lkml/2013/1/23/191
Reported-by: Martin Mokrejs <mmokrejs-08dBlVkRsZWoiTQjSSYKZesEoJ4y9sgM@public.gmane.org>
Tested-by: Peter Wu <lekensteyn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [ACPI conflict error path]
Signed-off-by: Peter Wu <lekensteyn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Hi Jean,

This memleak issue is still present in v3.13-rc4-256-gb7000ad.
>From kmemleak:

unreferenced object 0xffff88022f501a00 (size 256):
  comm "systemd-udevd", pid 209, jiffies 4294896115 (age 2872.520s)
  hex dump (first 32 bytes):
    00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00  .....N..........
    ff ff ff ff ff ff ff ff f4 e2 53 82 ff ff ff ff  ..........S.....
  backtrace:
    [<ffffffff815d29ce>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff8116ea5a>] kmem_cache_alloc_trace+0xfa/0x1e0
    [<ffffffff813efc63>] device_private_init+0x23/0x80
    [<ffffffff813f2b49>] dev_set_drvdata+0x39/0x50
    [<ffffffffa0294539>] i801_probe+0x59/0x528 [i2c_i801]
    [<ffffffff81332d95>] local_pci_probe+0x45/0xa0
    [<ffffffff81333be9>] pci_device_probe+0xd9/0x130
    [<ffffffff813f30e7>] driver_probe_device+0x87/0x390
    [<ffffffff813f34c3>] __driver_attach+0x93/0xa0
    [<ffffffff813f102b>] bus_for_each_dev+0x6b/0xb0
    [<ffffffff813f2b0e>] driver_attach+0x1e/0x20
    [<ffffffff813f26e8>] bus_add_driver+0x188/0x260
    [<ffffffff813f3b04>] driver_register+0x64/0xf0
    [<ffffffff81332930>] __pci_register_driver+0x60/0x70
    [<ffffffffa02990af>] 0xffffffffa02990af
    [<ffffffff81000312>] do_one_initcall+0xf2/0x1a0

The dmesg for this laptop also contains a resource conflict message,
just like the reporter (Martin Mokrejs):

    [   15.409772] ACPI Warning: 0x0000000000001840-0x000000000000185f SystemIO conflicts with Region \_SB_.PCI0.SBUS.SMBI 1 (20131115/utaddress-251)
    [   15.413439] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver

With this patch applied on top of almost 3.13-rc5 (v3.13-rc4-256-gb7000ad),
the memleak is gone.

Regards,
Peter
---
 drivers/i2c/busses/i2c-i801.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 737e298..a7096bf 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1117,6 +1117,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!priv)
 		return -ENOMEM;
 
+	pci_set_drvdata(dev, priv);
 	i2c_set_adapdata(&priv->adapter, priv);
 	priv->adapter.owner = THIS_MODULE;
 	priv->adapter.class = i801_get_adapter_class(priv);
@@ -1236,8 +1237,6 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* We ignore errors - multiplexing is optional */
 	i801_add_mux(priv);
 
-	pci_set_drvdata(dev, priv);
-
 	return 0;
 
 exit_free_irq:
-- 
1.8.5.2

  parent reply	other threads:[~2013-12-23  9:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-23 11:15 linux-3.7.[1,4]: kmemleak in i801_probe Martin Mokrejs
     [not found] ` <50FFC659.1090402-08dBlVkRsZWoiTQjSSYKZesEoJ4y9sgM@public.gmane.org>
2013-01-23 16:42   ` Jean Delvare
     [not found]     ` <20130123174204.00463f98-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-01-23 17:35       ` Martin Mokrejs
2013-12-23  9:39       ` Peter Wu [this message]
     [not found]         ` <1387791578-1372-1-git-send-email-lekensteyn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-12-23 10:43           ` [PATCH] i2c: i801: fix memleak on probe error Peter Wu
2013-12-23 10:51             ` Martin Mokrejs
     [not found]               ` <52B815A0.1060609-08dBlVkRsZWoiTQjSSYKZesEoJ4y9sgM@public.gmane.org>
2013-12-23 15:49                 ` How should dev_[gs]et_drvdata be used? (was: Re: [PATCH] i2c: i801: fix memleak on probe error) Peter Wu
2013-12-23 17:37                   ` Alex Williamson
     [not found]                     ` <1387820241.30327.105.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org>
2013-12-24  0:18                       ` How should dev_[gs]et_drvdata be used? Peter Wu
2013-12-24  1:51                         ` Alex Williamson
     [not found]                           ` <1387849869.30327.201.camel-xdHQ/5r00wBBDLzU/O5InQ@public.gmane.org>
2013-12-24  9:44                             ` Peter Wu
2014-01-08 13:28                         ` Jean Delvare
     [not found]                           ` <20140108142849.3993341c-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-11-25 21:14                             ` Uwe Kleine-König
     [not found]                               ` <20141125211432.GA6008-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-28 13:48                                 ` Jean Delvare
     [not found]                                   ` <20141128144813.3e6fd8d9-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-11-28 14:04                                     ` Uwe Kleine-König
2014-01-08  9:05             ` [PATCH] i2c: i801: fix memleak on probe error Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387791578-1372-1-git-send-email-lekensteyn@gmail.com \
    --to=lekensteyn-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mmokrejs-08dBlVkRsZWoiTQjSSYKZesEoJ4y9sgM@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).