Thread (2 messages) 2 messages, 2 authors, 2010-05-25

Re: [PATCH] touchscreen: ads7846: please don't touch free'd memory

From: Kevin Hilman <hidden>
Date: 2010-05-25 22:18:20
Also in: linux-omap, lkml
Subsystem: input (keyboard, mouse, joystick, touchscreen) drivers, the rest · Maintainers: Dmitry Torokhov, Linus Torvalds

Possibly related (same subject, not in this thread)

Mike Frysinger [off-list ref] writes:
On Tue, May 25, 2010 at 17:46, Kevin Hilman wrote:
quoted
After digging into the driver core and realizing that it seemed to
have sane error handling itself, I took a closer look at
ads7846_probe() and discovered it doesn't actually return an error
code for certain failure cases!  That was the root cause.
that is crappy
quoted
Subject: [PATCH] input: touchscreen: ads7846: return error on probe failure
i'd refer to the specific probe issue rather than just "probe".  maybe:
input: touchscreen: ads7846: return error on regulator_get() failure
Thanks for the review, here's one with updated subject and ack added.

Kevin

From 8ce49a91341d8713f870d2a931969f227a82b8ad Mon Sep 17 00:00:00 2001
From: Kevin Hilman <redacted>
Date: Tue, 25 May 2010 14:38:04 -0700
Subject: [PATCH] input: touchscreen: ads7846: return error on regulator_get() failure

In probe(), if regulator_get() failed, an error code was not being
returned causing the driver to be successfully bound, even though
probe failed.  This in turn caused the suspend, resume and remove
methods to be registered and accessed via the SPI core.  Since these
functions all access private driver data using pointers that had been
freed during the failed probe, this would lead to unpredictable
behavior.

This patch ensures that probe() returns an error code in this failure
case so the driver is not bound.

Found using lockdep and noticing the lock used in the suspend/resum
path pointed to a bogus lock due to the freed memory.

Signed-off-by: Kevin Hilman <redacted>
Acked-by: Mike Frysinger <redacted>
---
 drivers/input/touchscreen/ads7846.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 532279c..634f6f6 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1163,8 +1163,8 @@ static int __devinit ads7846_probe(struct spi_device *spi)
 
 	ts->reg = regulator_get(&spi->dev, "vcc");
 	if (IS_ERR(ts->reg)) {
-		dev_err(&spi->dev, "unable to get regulator: %ld\n",
-			PTR_ERR(ts->reg));
+		err = PTR_ERR(ts->reg);
+		dev_err(&spi->dev, "unable to get regulator: %ld\n", err);
 		goto err_free_gpio;
 	}
 
-- 
1.7.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help