2023-10-23 08:03:15

by Billy Tsai

Subject: [PATCH v1] i3c/master/mipi-i3c-hci: Fix a kernel panic for accessing DAT_data.

The `i3c_master_bus_init` function may attach the I2C devices before the
I3C bus initialization. In this flow, the DAT `alloc_entry`` will be used
before the DAT `init`. Additionally, if the `i3c_master_bus_init` fails,
the DAT `cleanup` will execute before the device is detached, which will
execue DAT `free_entry` function. The above scenario can cause the driver
to use DAT_data when it is NULL.

Signed-off-by: Billy Tsai <[email protected]>
---
drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 29 ++++++++++++++++--------
1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
index 97bb49ff5b53..47b9b4d4ed3f 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
@@ -64,15 +64,17 @@ static int hci_dat_v1_init(struct i3c_hci *hci)
return -EOPNOTSUPP;
}

- /* use a bitmap for faster free slot search */
- hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL);
- if (!hci->DAT_data)
- return -ENOMEM;
-
- /* clear them */
- for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) {
- dat_w0_write(dat_idx, 0);
- dat_w1_write(dat_idx, 0);
+ if (!hci->DAT_data) {
+ /* use a bitmap for faster free slot search */
+ hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL);
+ if (!hci->DAT_data)
+ return -ENOMEM;
+
+ /* clear them */
+ for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) {
+ dat_w0_write(dat_idx, 0);
+ dat_w1_write(dat_idx, 0);
+ }
}

return 0;
@@ -87,7 +89,13 @@ static void hci_dat_v1_cleanup(struct i3c_hci *hci)
static int hci_dat_v1_alloc_entry(struct i3c_hci *hci)
{
unsigned int dat_idx;
+ int ret;

+ if (!hci->DAT_data) {
+ ret = hci_dat_v1_init(hci);
+ if (ret)
+ return ret;
+ }
dat_idx = find_first_zero_bit(hci->DAT_data, hci->DAT_entries);
if (dat_idx >= hci->DAT_entries)
return -ENOENT;
@@ -103,7 +111,8 @@ static void hci_dat_v1_free_entry(struct i3c_hci *hci, unsigned int dat_idx)
{
dat_w0_write(dat_idx, 0);
dat_w1_write(dat_idx, 0);
- __clear_bit(dat_idx, hci->DAT_data);
+ if (hci->DAT_data)
+ __clear_bit(dat_idx, hci->DAT_data);
}

static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci,
--
2.25.1

2023-11-03 23:42:16

by Alexandre Belloni

Subject: Re: [PATCH v1] i3c/master/mipi-i3c-hci: Fix a kernel panic for accessing DAT_data.

On Mon, 23 Oct 2023 16:02:37 +0800, Billy Tsai wrote:
> The `i3c_master_bus_init` function may attach the I2C devices before the
> I3C bus initialization. In this flow, the DAT `alloc_entry`` will be used
> before the DAT `init`. Additionally, if the `i3c_master_bus_init` fails,
> the DAT `cleanup` will execute before the device is detached, which will
> execue DAT `free_entry` function. The above scenario can cause the driver
> to use DAT_data when it is NULL.
>
> [...]

Applied, thanks!

[1/1] i3c/master/mipi-i3c-hci: Fix a kernel panic for accessing DAT_data.
commit: b53e9758a31c683fc8615df930262192ed5f034b

Best regards,

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com