[PATCH] fix adm9240 oops

[PATCH] fix adm9240 oops

[Jonathan Corbet]

The adm9240 driver, in adm9240_detect(), allocates a structure.  The
error path attempts to kfree() a subfield of that structure, resulting
in an oops (or slab corruption) if the hardware is not present.  This
one seems worth fixing for 2.6.13.

jon

Signed-off-by: Jonathan Corbet <corbet@lwn.net>

--- 2.6.13-rc7/drivers/hwmon/adm9240.c.orig	2005-08-25 14:30:04.000000000 -0600
+++ 2.6.13-rc7/drivers/hwmon/adm9240.c	2005-08-25 14:30:26.000000000 -0600
@@ -616,7 +616,7 @@ static int adm9240_detect(struct i2c_ada
 
 	return 0;
 exit_free:
-	kfree(new_client);
+	kfree(data);
 exit:
 	return err;
 }

[PATCH] drivers/hwmon/*: kfree() correct pointers

[Alexey Dobriyan]

The adm9240 driver, in adm9240_detect(), allocates a structure.  The
error path attempts to kfree() ->client field of it (second one),
resulting in an oops (or slab corruption) if the hardware is not present.

->client field in adm1026, adm1031, smsc47b397 and smsc47m1 is the first in
${HWMON}_data structure, but fix them too.

Signed-off-by: Jonathan Corbet <corbet@lwn.net
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/hwmon/adm1026.c    |    2 +-
 drivers/hwmon/adm1031.c    |    2 +-
 drivers/hwmon/adm9240.c    |    2 +-
 drivers/hwmon/smsc47b397.c |    2 +-
 drivers/hwmon/smsc47m1.c   |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff -uprN linux-vanilla/drivers/hwmon/adm1026.c linux-hwmon/drivers/hwmon/adm1026.c
--- linux-vanilla/drivers/hwmon/adm1026.c	2005-08-25 18:57:18.000000000 +0400
+++ linux-hwmon/drivers/hwmon/adm1026.c	2005-08-26 01:16:07.000000000 +0400
@@ -1691,7 +1691,7 @@ int adm1026_detect(struct i2c_adapter *a
 
 	/* Error out and cleanup code */
 exitfree:
-	kfree(new_client);
+	kfree(data);
 exit:
 	return err;
 }
diff -uprN linux-vanilla/drivers/hwmon/adm1031.c linux-hwmon/drivers/hwmon/adm1031.c
--- linux-vanilla/drivers/hwmon/adm1031.c	2005-08-25 18:57:18.000000000 +0400
+++ linux-hwmon/drivers/hwmon/adm1031.c	2005-08-26 01:16:26.000000000 +0400
@@ -834,7 +834,7 @@ static int adm1031_detect(struct i2c_ada
 	return 0;
 
 exit_free:
-	kfree(new_client);
+	kfree(data);
 exit:
 	return err;
 }
diff -uprN linux-vanilla/drivers/hwmon/adm9240.c linux-hwmon/drivers/hwmon/adm9240.c
--- linux-vanilla/drivers/hwmon/adm9240.c	2005-08-25 18:57:18.000000000 +0400
+++ linux-hwmon/drivers/hwmon/adm9240.c	2005-08-26 01:16:40.000000000 +0400
@@ -616,7 +616,7 @@ static int adm9240_detect(struct i2c_ada
 
 	return 0;
 exit_free:
-	kfree(new_client);
+	kfree(data);
 exit:
 	return err;
 }
diff -uprN linux-vanilla/drivers/hwmon/smsc47b397.c linux-hwmon/drivers/hwmon/smsc47b397.c
--- linux-vanilla/drivers/hwmon/smsc47b397.c	2005-08-25 18:57:18.000000000 +0400
+++ linux-hwmon/drivers/hwmon/smsc47b397.c	2005-08-26 01:21:11.000000000 +0400
@@ -298,7 +298,7 @@ static int smsc47b397_detect(struct i2c_
 	return 0;
 
 error_free:
-	kfree(new_client);
+	kfree(data);
 error_release:
 	release_region(addr, SMSC_EXTENT);
 	return err;
diff -uprN linux-vanilla/drivers/hwmon/smsc47m1.c linux-hwmon/drivers/hwmon/smsc47m1.c
--- linux-vanilla/drivers/hwmon/smsc47m1.c	2005-08-25 18:57:18.000000000 +0400
+++ linux-hwmon/drivers/hwmon/smsc47m1.c	2005-08-26 01:21:28.000000000 +0400
@@ -495,7 +495,7 @@ static int smsc47m1_detect(struct i2c_ad
 	return 0;
 
 error_free:
-	kfree(new_client);
+	kfree(data);
 error_release:
 	release_region(address, SMSC_EXTENT);
 	return err;

Re: [PATCH] drivers/hwmon/*: kfree() correct pointers

[Jean Delvare]

Hi Alexey,

> The adm9240 driver, in adm9240_detect(), allocates a structure.  The
> error path attempts to kfree() ->client field of it (second one),
> resulting in an oops (or slab corruption) if the hardware is not
> present.
> 
> ->client field in adm1026, adm1031, smsc47b397 and smsc47m1 is the
> first in ${HWMON}_data structure, but fix them too.

Already fixed in Greg's i2c tree and -mm for quite some time now...

http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-02-i2c/i2c-hwmon-class-01.patch

Thanks,
-- 
Jean Delvare

Re: [PATCH] drivers/hwmon/*: kfree() correct pointers

[Jonathan Corbet]

> Already fixed in Greg's i2c tree and -mm for quite some time now...

So it is.  The comment says, however, that "the existing code works
somewhat by accident."  In the case of the 9240 driver, however, the
existing code demonstrably does not work - it oopsed on me.  The patch
in Greg's tree looks fine (it's a straightforward fix, after all); I'd
recommend that it be merged before 2.6.13.

jon

Jonathan Corbet
Executive editor, LWN.net
corbet@lwn.net

Re: [PATCH] drivers/hwmon/*: kfree() correct pointers

[Jean Delvare]

Hi Jonathan,

> > Already fixed in Greg's i2c tree and -mm for quite some time now...
> 
> So it is.  The comment says, however, that "the existing code works
> somewhat by accident."  In the case of the 9240 driver, however, the
> existing code demonstrably does not work - it oopsed on me.

I too did notice that the adm9240 case was worse than the four other
ones back then, but when I tried to get it to crash, it never did.  This
is the reason why I did not push this patch upstream faster.  I wonder
why it now does oops on you.

I also believe that this patch was somewhat misnamed.  It is not related
to the new hwmon class, but jut happened to change the same part of
these five drivers.  With a better name, the patch would most probably
have been selected by Greg in the last batch of i2c patches to Linus.

> The patch in Greg's tree looks fine (it's a straightforward fix, after
> all);

I wouldn't call it straightforward, but it certainly has been reviewed
and tested well enough by now to be considered safe.

> I'd recommend that it be merged before 2.6.13.

Fine with me.

Thanks,
-- 
Jean Delvare