2021-04-15 09:39:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] i2c: s3c2410: simplify getting of_device_id match data

Use of_device_get_match_data() to make the code slightly smaller.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/i2c/busses/i2c-s3c2410.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 62a903fbe912..ab928613afba 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/gpio/consumer.h>
#include <linux/pinctrl/consumer.h>
#include <linux/mfd/syscon.h>
@@ -156,12 +157,8 @@ MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
*/
static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev)
{
- if (pdev->dev.of_node) {
- const struct of_device_id *match;
-
- match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
- return (kernel_ulong_t)match->data;
- }
+ if (pdev->dev.of_node)
+ return (kernel_ulong_t)of_device_get_match_data(&pdev->dev);

return platform_get_device_id(pdev)->driver_data;
}
--
2.25.1


2021-04-15 09:41:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [RFT 2/2] i2c: s3c2410: fix possible NULL pointer deref on read message after write

Interrupt handler processes multiple message write requests one after
another, till the driver message queue is drained. However if driver
encounters a read message without preceding START, it stops the I2C
transfer as it is an invalid condition for the controller. At least the
comment describes a requirement "the controller forces us to send a new
START when we change direction". This stop results in clearing the
message queue (i2c->msg = NULL).

The code however immediately jumped back to label "retry_write" which
dereferenced the "i2c->msg" making it a possible NULL pointer
dereference.

The Coverity analysis:
1. Condition !is_msgend(i2c), taking false branch.
if (!is_msgend(i2c)) {

2. Condition !is_lastmsg(i2c), taking true branch.
} else if (!is_lastmsg(i2c)) {

3. Condition i2c->msg->flags & 1, taking true branch.
if (i2c->msg->flags & I2C_M_RD) {

4. write_zero_model: Passing i2c to s3c24xx_i2c_stop, which sets i2c->msg to NULL.
s3c24xx_i2c_stop(i2c, -EINVAL);

5. Jumping to label retry_write.
goto retry_write;

All previous calls to s3c24xx_i2c_stop() in this interrupt service
routine are followed by jumping to end of function (acknowledging
the interrupt and returning). This seems a reasonable choice also here
since message buffer was entirely emptied.

Addresses-Coverity: Explicit null dereferenced
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/i2c/busses/i2c-s3c2410.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index ab928613afba..4d82761e1585 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -480,7 +480,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat)
* forces us to send a new START
* when we change direction
*/
+ dev_dbg(i2c->dev,
+ "missing START before write->read\n");
s3c24xx_i2c_stop(i2c, -EINVAL);
+ break;
}

goto retry_write;
--
2.25.1

2021-04-15 09:47:47

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] i2c: s3c2410: simplify getting of_device_id match data


On 15.04.2021 11:38, Krzysztof Kozlowski wrote:
> Use of_device_get_match_data() to make the code slightly smaller.
>
> Signed-off-by: Krzysztof Kozlowski<[email protected]>

Reviewed-by: Sylwester Nawrocki <[email protected]>

2021-04-16 23:08:32

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 1/2] i2c: s3c2410: simplify getting of_device_id match data

On Thu, Apr 15, 2021 at 11:38:02AM +0200, Krzysztof Kozlowski wrote:
> Use of_device_get_match_data() to make the code slightly smaller.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>

Applied to for-next, thanks!


Attachments:
(No filename) (251.00 B)
signature.asc (849.00 B)
Download all attachments