Bug 9019 - i2c: smbus:NULL function pointer dereference
Summary: i2c: smbus:NULL function pointer dereference
Status: NEW
Alias: None
Product: ANCK 5.10 Dev
Classification: ANCK
Component: drivers (show other bugs) drivers
Version: unspecified
Hardware: All Linux
: P3-Medium S3-normal
Target Milestone: ---
Assignee: GuixinLiu
QA Contact: shuming
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-14 11:15 UTC by caulif
Modified: 2024-05-14 15:35 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description caulif 2024-05-14 11:15:25 UTC
Description of problem:
Running i2cdetect on i2c slave device (i2c-designware) results in a kernel NULL pointer dereference, causing an internal error and crash.

How reproducible:
Always

Steps to Reproduce:
1. Run i2cdetect on an i2c slave device using i2c-designware.


Actual results:
Kernel NULL pointer dereference at virtual address 0000000000000000, resulting in an internal error and system crash.

Expected results:
i2cdetect runs without causing a kernel NULL pointer dereference and does not crash the system.

Additional info:
Callers of __i2c_transfer() must verify that master_xfer is not NULL.
Comment 1 小龙 admin 2024-05-14 15:08:43 UTC
The PR Link: https://gitee.com/anolis/cloud-kernel/pulls/3171
Comment 2 caulif 2024-05-14 15:31:21 UTC
commit 91811a31b68d3765b3065f4bb6d7d6d84a7cfc9f
Category: bugfix
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=91811a31b68d3765b3065f4bb6d7d6d84a7cfc9f

There is an issue where the designware controller,
when used in target-onlymode, leads to an OOPS.
This breaks the assumption of one transfer function
always being available.
The problem stems from a NULL function pointer dereference
in the __i2c_transfer function.
To address this issue, I added a check to ensure that
the function pointer is not NULL before calling it.

Signed-off-by: caulif <1589375811@qq.com>

 drivers/i2c/i2c-core-base.c  | 11 ++++++-----
 drivers/i2c/i2c-core-smbus.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

```diff
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index bdce6d3e5327..7b6134edcb76 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2013,13 +2013,18 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
  * Returns negative errno, else the number of messages executed.
  *
  * Adapter lock must be held when calling this function. No debug logging
- * takes place. adap->algo->master_xfer existence isn't checked.
+ * takes place.
  */
 int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	unsigned long orig_jiffies;
 	int ret, try;
 
+	if (!adap->algo->master_xfer) {
+		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (WARN_ON(!msgs || num < 1))
 		return -EINVAL;
 
@@ -2086,10 +2091,6 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	int ret;
 
-	if (!adap->algo->master_xfer) {
-		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
-		return -EOPNOTSUPP;
-	}
 
 	/* REVISIT the fault reporting model here is weak:
 	 *
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index f5c9787992e9..465ce83f222c 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -593,7 +593,7 @@ s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 				break;
 		}
 
-		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
+		if (res != -EOPNOTSUPP)
 			goto trace;
 		/*
 		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't
```
Comment 3 caulif 2024-05-14 15:35:43 UTC
commit 91811a31b68d3765b3065f4bb6d7d6d84a7cfc9f
Category: bugfix
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=91811a31b68d3765b3065f4bb6d7d6d84a7cfc9f

There is an issue where the designware controller,
when used in target-onlymode, leads to an OOPS.
This breaks the assumption of one transfer function
always being available.
The problem stems from a NULL function pointer dereference
in the __i2c_transfer function.
To address this issue, I added a check to ensure that
the function pointer is not NULL before calling it.

Signed-off-by: caulif <1589375811@qq.com>

 drivers/i2c/i2c-core-base.c  | 11 ++++++-----
 drivers/i2c/i2c-core-smbus.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

```diff
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index bdce6d3e5327..7b6134edcb76 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2013,13 +2013,18 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
  * Returns negative errno, else the number of messages executed.
  *
  * Adapter lock must be held when calling this function. No debug logging
- * takes place. adap->algo->master_xfer existence isn't checked.
+ * takes place.
  */
 int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	unsigned long orig_jiffies;
 	int ret, try;
 
+	if (!adap->algo->master_xfer) {
+		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (WARN_ON(!msgs || num < 1))
 		return -EINVAL;
 
@@ -2086,10 +2091,6 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	int ret;
 
-	if (!adap->algo->master_xfer) {
-		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
-		return -EOPNOTSUPP;
-	}
 
 	/* REVISIT the fault reporting model here is weak:
 	 *
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index f5c9787992e9..465ce83f222c 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -593,7 +593,7 @@ s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 				break;
 		}
 
-		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
+		if (res != -EOPNOTSUPP)
 			goto trace;
 		/*
 		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't
```