[PATCH 0/2] base/platform: Platform data and resources NULL handling

[PATCH 0/2] base/platform: Platform data and resources NULL handling

[Anton Vorontsov]

Hi all,

There is a bug in the MFD core code (drivers/mfd/mfd-core.c),
the mfd_add_device function does not check platform_data for NULL,
and thus always calls platform_device_add_data():

static int mfd_add_device(struct device *parent, int id, [...])
{
	[...]
	ret = platform_device_add_data(pdev,
		cell->platform_data, cell->data_size);
	[...]
}

The problem is that when cell->platform_data is NULL, the platform
core calls kmemdup(NULL, 0, ...), which returns a non-NULL result
(ZERO_SIZE_PTR), and the result is stored in the dev.platform_data.
This causes drivers to oops on a valid code:

if (pdata)
	stuff = pdata->stuff;

Firstly I thought that I would fix the MFD core, but it appears
that the better approach would be to change device_add_data() call
behaviour, and thus make the core code more safe.

There are two patches: a patch that is necessary for the bug fix,
and a non-essential cleanup patch.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/2] base/platform: Safe handling for NULL platform data and resources

[Anton Vorontsov]

Some users of platform_device_add_{data,resources}() assume that
NULL data and resources will be handled specially, i.e. just ignored.

But the platform core ends up calling kmemdup(NULL, 0, ...), which
returns a non-NULL result (i.e. ZERO_SIZE_PTR), which causes drivers
to oops on a valid code, something like:

  if (platform_data)
  	stuff = platform_data->stuff;

This patch makes the platform core a bit more safe for such cases.

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/base/platform.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c6c933f..67519cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -192,6 +192,9 @@ int platform_device_add_resources(struct platform_device *pdev,
 {
 	struct resource *r;
 
+	if (!res)
+		return 0;
+
 	r = kmemdup(res, sizeof(struct resource) * num, GFP_KERNEL);
 	if (r) {
 		pdev->resource = r;
@@ -215,8 +218,12 @@ EXPORT_SYMBOL_GPL(platform_device_add_resources);
 int platform_device_add_data(struct platform_device *pdev, const void *data,
 			     size_t size)
 {
-	void *d = kmemdup(data, size, GFP_KERNEL);
+	void *d;
+
+	if (!data)
+		return 0;
 
+	d = kmemdup(data, size, GFP_KERNEL);
 	if (d) {
 		pdev->dev.platform_data = d;
 		return 0;
-- 
1.7.0.5

[PATCH 2/2] base/platform: Simplifications for NULL platform data/resources handling

[Anton Vorontsov]

There's no need to explicitly check for data and resources being NULL,
as platform_device_add_{data,resources}() do this internally nowadays.

This makes the code more linear and less indented.

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/base/platform.c |   32 ++++++++++++--------------------
 1 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 67519cd..716d563 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -380,17 +380,13 @@ struct platform_device *__init_or_module platform_device_register_resndata(
 
 	pdev->dev.parent = parent;
 
-	if (res) {
-		ret = platform_device_add_resources(pdev, res, num);
-		if (ret)
-			goto err;
-	}
+	ret = platform_device_add_resources(pdev, res, num);
+	if (ret)
+		goto err;
 
-	if (data) {
-		ret = platform_device_add_data(pdev, data, size);
-		if (ret)
-			goto err;
-	}
+	ret = platform_device_add_data(pdev, data, size);
+	if (ret)
+		goto err;
 
 	ret = platform_device_add(pdev);
 	if (ret) {
@@ -537,17 +533,13 @@ struct platform_device * __init_or_module platform_create_bundle(
 		goto err_out;
 	}
 
-	if (res) {
-		error = platform_device_add_resources(pdev, res, n_res);
-		if (error)
-			goto err_pdev_put;
-	}
+	error = platform_device_add_resources(pdev, res, n_res);
+	if (error)
+		goto err_pdev_put;
 
-	if (data) {
-		error = platform_device_add_data(pdev, data, size);
-		if (error)
-			goto err_pdev_put;
-	}
+	error = platform_device_add_data(pdev, data, size);
+	if (error)
+		goto err_pdev_put;
 
 	error = platform_device_add(pdev);
 	if (error)
-- 
1.7.0.5