From: Sicong Huang <congei42(a)163.com>
[ Upstream commit 5c9c5d7f26acc2c669c1dcf57d1bb43ee99220ce ]
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
Link: https://lore.kernel.org/r/20240416080313.92306-1-congei42@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index 9ec949a438ef6..52ef6be9d4499 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -694,6 +694,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.43.0
From: Sicong Huang <congei42(a)163.com>
[ Upstream commit 5c9c5d7f26acc2c669c1dcf57d1bb43ee99220ce ]
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
Link: https://lore.kernel.org/r/20240416080313.92306-1-congei42@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index 9ec949a438ef6..52ef6be9d4499 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -694,6 +694,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.43.0
From: Sicong Huang <congei42(a)163.com>
[ Upstream commit 5c9c5d7f26acc2c669c1dcf57d1bb43ee99220ce ]
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
Link: https://lore.kernel.org/r/20240416080313.92306-1-congei42@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index 9ec949a438ef6..52ef6be9d4499 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -694,6 +694,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.43.0
From: Sicong Huang <congei42(a)163.com>
[ Upstream commit 5c9c5d7f26acc2c669c1dcf57d1bb43ee99220ce ]
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
Link: https://lore.kernel.org/r/20240416080313.92306-1-congei42@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index 9ec949a438ef6..52ef6be9d4499 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -694,6 +694,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.43.0
From: Sicong Huang <congei42(a)163.com>
[ Upstream commit 5c9c5d7f26acc2c669c1dcf57d1bb43ee99220ce ]
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
Link: https://lore.kernel.org/r/20240416080313.92306-1-congei42@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index fd58a86b0888d..d022bfb5e95d7 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -693,6 +693,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.43.0
In gb_interface_create, &intf->mode_switch_completion is bound with
gb_interface_mode_switch_work. Then it will be started by
gb_interface_request_mode_switch. Here is the relevant code.
if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
...
}
If we call gb_interface_release to make cleanup, there may be an
unfinished work. This function will call kfree to free the object
"intf". However, if gb_interface_mode_switch_work is scheduled to
run after kfree, it may cause use-after-free error as
gb_interface_mode_switch_work will use the object "intf".
The possible execution flow that may lead to the issue is as follows:
CPU0 CPU1
| gb_interface_create
| gb_interface_request_mode_switch
gb_interface_release |
kfree(intf) (free) |
| gb_interface_mode_switch_work
| mutex_lock(&intf->mutex) (use)
Fix it by canceling the work before kfree.
Signed-off-by: Sicong Huang <congei42(a)163.com>
---
drivers/greybus/interface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index fd58a86b0888..d022bfb5e95d 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -693,6 +693,7 @@ static void gb_interface_release(struct device *dev)
trace_gb_interface_release(intf);
+ cancel_work_sync(&intf->mode_switch_work);
kfree(intf);
}
--
2.34.1
On Tue, Apr 16, 2024 at 11:00:25AM +0800, sicong wrote:
> greybus/interface.c: use-after-free bug in gb_interface_release due to
> race condition.
>
> In gb_interface_create, &intf->mode_switch_completion is bound with
> gb_interface_mode_switch_work. Then it will be started by
> gb_interface_request_mode_switch. Here is the code.
> if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
> ...
> }
>
> If we call gb_interface_release to make cleanup, there may be an
> unfinished work. This function will call kfree to free the object
> "intf". However, if gb_interface_mode_switch_work is scheduled to
> run after kfree, it may cause use-after-free error as
> gb_interface_mode_switch_work will use the object "intf".
> The possible execution flow that may lead to the issue is as follows:
>
> CPU0 CPU1
>
> | gb_interface_create
> | gb_interface_request_mode_switch
> gb_interface_release |
> kfree(intf) (free) |
> | gb_interface_mode_switch_work
> | mutex_lock(&intf->mutex) (use)
>
> This bug may be fixed by adding the following code before kfree.
> cancel_work_sync(&intf->mode_switch_work);
Wonderful, please submit a patch with this information and we will be
glad to review it.
thanks,
greg k-h
MikroBUS is an open standard developed by MikroElektronika for connecting
add-on boards to microcontrollers or microprocessors. It essentially
allows you to easily expand the functionality of your main boards using
these add-on boards.
This patchset adds mikroBUS as a Linux bus type and provides a driver to
parse, and flash mikroBUS manifest and register the mikroBUS board.
The v1 and v2 of this patchset was submitted by Vaishnav M A back in
2020. This patchset also includes changes made over the years as part of
BeagleBoards kernel.
Link: https://www.mikroe.com/mikrobus
Link: https://docs.beagleboard.org/latest/boards/beagleplay/
Link: https://lore.kernel.org/lkml/20200818124815.11029-1-vaishnav@beagleboard.or… Patch v2
Changes in v3:
- Use phandle instead of busname for spi
- Use spi board info for registering new device
- Convert dt bindings to yaml
- Add support for clickID
- Code cleanup and style changes
- Additions required to spi, serdev, w1 and regulator subsystems
Changes in v2:
- support for adding mikroBUS ports from DT overlays,
- remove debug sysFS interface for adding mikrobus ports,
- consider extended pin usage/deviations from mikrobus standard
specifications
- use greybus CPort protocol enum instead of new protocol enums
- Fix cases of wrong indentation, ignoring return values, freeing allocated
resources in case of errors and other style suggestions in v1 review.
Ayush Singh (7):
dt-bindings: misc: Add mikrobus-connector
w1: Add w1_find_master_device
spi: Make of_find_spi_controller_by_node() available
regulator: fixed-helper: export regulator_register_always_on
greybus: Add mikroBUS manifest types
mikrobus: Add mikrobus driver
dts: ti: k3-am625-beagleplay: Add mikroBUS
Vaishnav M A (1):
serdev: add of_ helper to get serdev controller
.../bindings/misc/mikrobus-connector.yaml | 110 ++
MAINTAINERS | 7 +
.../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +-
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/mikrobus/Kconfig | 19 +
drivers/misc/mikrobus/Makefile | 6 +
drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++
drivers/misc/mikrobus/mikrobus_core.h | 201 ++++
drivers/misc/mikrobus/mikrobus_id.c | 229 +++++
drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++
drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
drivers/regulator/fixed-helper.c | 1 +
drivers/spi/spi.c | 206 ++--
drivers/tty/serdev/core.c | 19 +
drivers/w1/w1.c | 6 +-
drivers/w1/w1_int.c | 27 +
include/linux/greybus/greybus_manifest.h | 49 +
include/linux/serdev.h | 4 +
include/linux/spi/spi.h | 4 +
include/linux/w1.h | 1 +
21 files changed, 2318 insertions(+), 113 deletions(-)
create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
create mode 100644 drivers/misc/mikrobus/Kconfig
create mode 100644 drivers/misc/mikrobus/Makefile
create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
--
2.44.0