[v1] i2c: core: Protect acpi_match_device() against NULL

Message ID 20170709131618.12121-1-andriy.shevchenko@linux.intel.com
State Superseded
Headers show

Commit Message

Andy Shevchenko July 9, 2017, 1:16 p.m.
The commit 09b3b246acba
	("i2c: core: Allow empty id_table in ACPI case as well")
brought an additional check for ACPI match table entry, though missed
NULL check for table itself.

Do it here by introducing a helper i2c_acpi_match_device().

Note, we rename some static function in i2c-core-acpi.c to distinguish
with public API.

Fixes: 09b3b246acba ("i2c: core: Allow empty id_table in ACPI case as well")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/i2c-core-acpi.c | 19 +++++++++++++++----
 drivers/i2c/i2c-core-base.c |  2 +-
 drivers/i2c/i2c-core.h      |  9 +++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

Comments

Andy Shevchenko July 9, 2017, 1:18 p.m. | #1
On Sun, 2017-07-09 at 16:16 +0300, Andy Shevchenko wrote:
> The commit 09b3b246acba
> 	("i2c: core: Allow empty id_table in ACPI case as well")
> brought an additional check for ACPI match table entry, though missed
> NULL check for table itself.
> 
> Do it here by introducing a helper i2c_acpi_match_device().
> 
> Note, we rename some static function in i2c-core-acpi.c to distinguish
> with public API.
> 

Wolfram, feel free to squash it into the original commit if it looks
better. 

> Fixes: 09b3b246acba ("i2c: core: Allow empty id_table in ACPI case as
> well")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/i2c-core-acpi.c | 19 +++++++++++++++----
>  drivers/i2c/i2c-core-base.c |  2 +-
>  drivers/i2c/i2c-core.h      |  9 +++++++++
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 4842ec3a5451..a9126b3cda61 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -230,6 +230,16 @@ void i2c_acpi_register_devices(struct i2c_adapter
> *adap)
>  		dev_warn(&adap->dev, "failed to enumerate I2C
> slaves\n");
>  }
>  
> +const struct acpi_device_id *
> +i2c_acpi_match_device(const struct acpi_device_id *matches,
> +		      struct i2c_client *client)
> +{
> +	if (!(client && matches))
> +		return NULL;
> +
> +	return acpi_match_device(matches, &client->dev);
> +}
> +
>  static acpi_status i2c_acpi_lookup_speed(acpi_handle handle, u32
> level,
>  					   void *data, void
> **return_value)
>  {
> @@ -289,7 +299,7 @@ u32 i2c_acpi_find_bus_speed(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(i2c_acpi_find_bus_speed);
>  
> -static int i2c_acpi_match_adapter(struct device *dev, void *data)
> +static int i2c_acpi_find_match_adapter(struct device *dev, void
> *data)
>  {
>  	struct i2c_adapter *adapter = i2c_verify_adapter(dev);
>  
> @@ -299,7 +309,7 @@ static int i2c_acpi_match_adapter(struct device
> *dev, void *data)
>  	return ACPI_HANDLE(dev) == (acpi_handle)data;
>  }
>  
> -static int i2c_acpi_match_device(struct device *dev, void *data)
> +static int i2c_acpi_find_match_device(struct device *dev, void *data)
>  {
>  	return ACPI_COMPANION(dev) == data;
>  }
> @@ -309,7 +319,7 @@ static struct i2c_adapter
> *i2c_acpi_find_adapter_by_handle(acpi_handle handle)
>  	struct device *dev;
>  
>  	dev = bus_find_device(&i2c_bus_type, NULL, handle,
> -			      i2c_acpi_match_adapter);
> +			      i2c_acpi_find_match_adapter);
>  	return dev ? i2c_verify_adapter(dev) : NULL;
>  }
>  
> @@ -317,7 +327,8 @@ static struct i2c_client
> *i2c_acpi_find_client_by_adev(struct acpi_device *adev)
>  {
>  	struct device *dev;
>  
> -	dev = bus_find_device(&i2c_bus_type, NULL, adev,
> i2c_acpi_match_device);
> +	dev = bus_find_device(&i2c_bus_type, NULL, adev,
> +			      i2c_acpi_find_match_device);
>  	return dev ? i2c_verify_client(dev) : NULL;
>  }
>  
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 45231d2257ad..ba51551c3077 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -357,7 +357,7 @@ static int i2c_device_probe(struct device *dev)
>  	 * Tree or ACPI match table entry is supplied for the probing
> device.
>  	 */
>  	if (!driver->id_table &&
> -	    !acpi_match_device(dev->driver->acpi_match_table,
> &client->dev) &&
> +	    !i2c_acpi_match_device(dev->driver->acpi_match_table,
> client) &&
>  	    !i2c_of_match_device(dev->driver->of_match_table,
> client))
>  		return -ENODEV;
>  
> diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
> index 3b63f5e5b89c..3d3d9bf02101 100644
> --- a/drivers/i2c/i2c-core.h
> +++ b/drivers/i2c/i2c-core.h
> @@ -31,9 +31,18 @@ int i2c_check_addr_validity(unsigned addr, unsigned
> short flags);
>  int i2c_check_7bit_addr_validity_strict(unsigned short addr);
>  
>  #ifdef CONFIG_ACPI
> +const struct acpi_device_id *
> +i2c_acpi_match_device(const struct acpi_device_id *matches,
> +		      struct i2c_client *client);
>  void i2c_acpi_register_devices(struct i2c_adapter *adap);
>  #else /* CONFIG_ACPI */
>  static inline void i2c_acpi_register_devices(struct i2c_adapter
> *adap) { }
> +static inline const struct acpi_device_id *
> +i2c_acpi_match_device(const struct acpi_device_id *matches,
> +		      struct i2c_client *client)
> +{
> +	return NULL;
> +}
>  #endif /* CONFIG_ACPI */
>  extern struct notifier_block i2c_acpi_notifier;
>
Wolfram Sang July 10, 2017, 6:21 p.m. | #2
On Sun, Jul 09, 2017 at 04:18:19PM +0300, Andy Shevchenko wrote:
> On Sun, 2017-07-09 at 16:16 +0300, Andy Shevchenko wrote:
> > The commit 09b3b246acba
> > 	("i2c: core: Allow empty id_table in ACPI case as well")
> > brought an additional check for ACPI match table entry, though missed
> > NULL check for table itself.
> > 
> > Do it here by introducing a helper i2c_acpi_match_device().
> > 
> > Note, we rename some static function in i2c-core-acpi.c to distinguish
> > with public API.
> > 
> 
> Wolfram, feel free to squash it into the original commit if it looks
> better. 

I didn't want to send Linus a branch which too many last minute fixes.
That's why I dropped the original patch and would like to ask you to
resend a combined patch after rc1. If it is a bugfix, we will have no
problems getting it into v4.13 but it seems a bit more testing is
needed.
Andy Shevchenko July 11, 2017, 10:14 a.m. | #3
On Mon, 2017-07-10 at 20:21 +0200, Wolfram Sang wrote:
> On Sun, Jul 09, 2017 at 04:18:19PM +0300, Andy Shevchenko wrote:
> > On Sun, 2017-07-09 at 16:16 +0300, Andy Shevchenko wrote:
> > > The commit 09b3b246acba
> > > 	("i2c: core: Allow empty id_table in ACPI case as well")
> > > brought an additional check for ACPI match table entry, though
> > > missed
> > > NULL check for table itself.
> > > 
> > > Do it here by introducing a helper i2c_acpi_match_device().
> > > 
> > > Note, we rename some static function in i2c-core-acpi.c to
> > > distinguish
> > > with public API.
> > > 
> > 
> > Wolfram, feel free to squash it into the original commit if it looks
> > better. 
> 
> I didn't want to send Linus a branch which too many last minute fixes.
> That's why I dropped the original patch and would like to ask you to
> resend a combined patch after rc1. If it is a bugfix, we will have no
> problems getting it into v4.13 but it seems a bit more testing is
> needed.

Okay, let's do this way.

I may also try to convert existing users (some coccinelle script I
suppose).

Patch

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 4842ec3a5451..a9126b3cda61 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -230,6 +230,16 @@  void i2c_acpi_register_devices(struct i2c_adapter *adap)
 		dev_warn(&adap->dev, "failed to enumerate I2C slaves\n");
 }
 
+const struct acpi_device_id *
+i2c_acpi_match_device(const struct acpi_device_id *matches,
+		      struct i2c_client *client)
+{
+	if (!(client && matches))
+		return NULL;
+
+	return acpi_match_device(matches, &client->dev);
+}
+
 static acpi_status i2c_acpi_lookup_speed(acpi_handle handle, u32 level,
 					   void *data, void **return_value)
 {
@@ -289,7 +299,7 @@  u32 i2c_acpi_find_bus_speed(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_acpi_find_bus_speed);
 
-static int i2c_acpi_match_adapter(struct device *dev, void *data)
+static int i2c_acpi_find_match_adapter(struct device *dev, void *data)
 {
 	struct i2c_adapter *adapter = i2c_verify_adapter(dev);
 
@@ -299,7 +309,7 @@  static int i2c_acpi_match_adapter(struct device *dev, void *data)
 	return ACPI_HANDLE(dev) == (acpi_handle)data;
 }
 
-static int i2c_acpi_match_device(struct device *dev, void *data)
+static int i2c_acpi_find_match_device(struct device *dev, void *data)
 {
 	return ACPI_COMPANION(dev) == data;
 }
@@ -309,7 +319,7 @@  static struct i2c_adapter *i2c_acpi_find_adapter_by_handle(acpi_handle handle)
 	struct device *dev;
 
 	dev = bus_find_device(&i2c_bus_type, NULL, handle,
-			      i2c_acpi_match_adapter);
+			      i2c_acpi_find_match_adapter);
 	return dev ? i2c_verify_adapter(dev) : NULL;
 }
 
@@ -317,7 +327,8 @@  static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev)
 {
 	struct device *dev;
 
-	dev = bus_find_device(&i2c_bus_type, NULL, adev, i2c_acpi_match_device);
+	dev = bus_find_device(&i2c_bus_type, NULL, adev,
+			      i2c_acpi_find_match_device);
 	return dev ? i2c_verify_client(dev) : NULL;
 }
 
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 45231d2257ad..ba51551c3077 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -357,7 +357,7 @@  static int i2c_device_probe(struct device *dev)
 	 * Tree or ACPI match table entry is supplied for the probing device.
 	 */
 	if (!driver->id_table &&
-	    !acpi_match_device(dev->driver->acpi_match_table, &client->dev) &&
+	    !i2c_acpi_match_device(dev->driver->acpi_match_table, client) &&
 	    !i2c_of_match_device(dev->driver->of_match_table, client))
 		return -ENODEV;
 
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 3b63f5e5b89c..3d3d9bf02101 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -31,9 +31,18 @@  int i2c_check_addr_validity(unsigned addr, unsigned short flags);
 int i2c_check_7bit_addr_validity_strict(unsigned short addr);
 
 #ifdef CONFIG_ACPI
+const struct acpi_device_id *
+i2c_acpi_match_device(const struct acpi_device_id *matches,
+		      struct i2c_client *client);
 void i2c_acpi_register_devices(struct i2c_adapter *adap);
 #else /* CONFIG_ACPI */
 static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { }
+static inline const struct acpi_device_id *
+i2c_acpi_match_device(const struct acpi_device_id *matches,
+		      struct i2c_client *client)
+{
+	return NULL;
+}
 #endif /* CONFIG_ACPI */
 extern struct notifier_block i2c_acpi_notifier;