Re: [crash, bisected] I2C, kobject (ffff81003e8c4160): tried to init an initialized object

From: Jean Delvare
Date: Wed Aug 06 2008 - 16:22:27 EST


Hi Alexey, Ingo,

On Thu, 17 Jul 2008 13:48:58 +0400, Alexey Dobriyan wrote:
> On Thu, Jul 17, 2008 at 11:33:30AM +0200, Ingo Molnar wrote:
> >
> > * Ingo Molnar <mingo@xxxxxxx> wrote:
> >
> > > Jean,
> > >
> > > -tip testing found another (similar) i2c crash with latest -git, on a
> > > Core2Duo laptop:
> > >
> > > calling nforce2_init+0x0/0x1b
> > > initcall nforce2_init+0x0/0x1b returned 0 after 0 msecs
> > > calling nforce2_s4985_init+0x0/0x275
> > > BUG: unable to handle kernel NULL pointer dereference at 0000000c
> > > IP: [<c04af2cf>] i2c_smbus_xfer+0x27/0x3f0
> > > *pdpt = 00000000009cb001 *pde = 0000000000000000
> > > Oops: 0000 [#1] SMP
> >
> > turning off I2C_NFORCE2_S4985 makes the system boot up fine. Workaround
> > patch below.
>
> Well, it's pretty easy oops unless I misread something.
>
> It happened on not nforce2 motherboard, right?
>
> nforce2_init will just register PCI driver,
> no matching PCI ids => nforce2_set_reference() isn't called =>
> nforce2_smbus stays NULL.
>
> Second module loads and tries to i2c_smbus_xfer(NULL, ) in module_init()
> hook which oopses.

Analysis is totally correct. I guess I shouldn't have tried to fix the
original bug in a hurry before leaving for vacation. I didn't have the
time to completely test it and as a result I introduced a new bug.
Which the following patch should fix:

* * * * *

Subject: i2c: Fix oops on bus multiplexer driver loading

The two I2C bus multiplexer drivers (i2c-amd756-s4882 and
i2c-nforce2-s4985) make use of the bus they want to multiplex before
checking if it is really present. Swap the instructions to test for
presence first. This fixes a oops reported by Ingo Molnar.

Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxx>
---
drivers/i2c/busses/i2c-amd756-s4882.c | 9 ++++-----
drivers/i2c/busses/i2c-nforce2-s4985.c | 5 +++--
2 files changed, 7 insertions(+), 7 deletions(-)

--- linux-2.6.27-rc2.orig/drivers/i2c/busses/i2c-amd756-s4882.c 2008-08-05 18:17:05.000000000 +0200
+++ linux-2.6.27-rc2/drivers/i2c/busses/i2c-amd756-s4882.c 2008-08-06 20:40:16.000000000 +0200
@@ -155,6 +155,9 @@ static int __init amd756_s4882_init(void
int i, error;
union i2c_smbus_data ioconfig;

+ if (!amd756_smbus.dev.parent)
+ return -ENODEV;
+
/* Configure the PCA9556 multiplexer */
ioconfig.byte = 0x00; /* All I/O to output mode */
error = i2c_smbus_xfer(&amd756_smbus, 0x18, 0, I2C_SMBUS_WRITE, 0x03,
@@ -168,11 +171,7 @@ static int __init amd756_s4882_init(void
/* Unregister physical bus */
error = i2c_del_adapter(&amd756_smbus);
if (error) {
- if (error == -EINVAL)
- error = -ENODEV;
- else
- dev_err(&amd756_smbus.dev, "Physical bus removal "
- "failed\n");
+ dev_err(&amd756_smbus.dev, "Physical bus removal failed\n");
goto ERROR0;
}

--- linux-2.6.27-rc2.orig/drivers/i2c/busses/i2c-nforce2-s4985.c 2008-08-05 18:17:05.000000000 +0200
+++ linux-2.6.27-rc2/drivers/i2c/busses/i2c-nforce2-s4985.c 2008-08-06 20:07:48.000000000 +0200
@@ -150,6 +150,9 @@ static int __init nforce2_s4985_init(voi
int i, error;
union i2c_smbus_data ioconfig;

+ if (!nforce2_smbus)
+ return -ENODEV;
+
/* Configure the PCA9556 multiplexer */
ioconfig.byte = 0x00; /* All I/O to output mode */
error = i2c_smbus_xfer(nforce2_smbus, 0x18, 0, I2C_SMBUS_WRITE, 0x03,
@@ -161,8 +164,6 @@ static int __init nforce2_s4985_init(voi
}

/* Unregister physical bus */
- if (!nforce2_smbus)
- return -ENODEV;
error = i2c_del_adapter(nforce2_smbus);
if (error) {
dev_err(&nforce2_smbus->dev, "Physical bus removal failed\n");

This is going to Linus tomorrow.

Thanks,
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/