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
From: Arnd Bergmann <arnd(a)arndb.de>
gcc-10 warns about a strncpy() that does not enforce zero-termination:
In file included from include/linux/string.h:369,
from drivers/staging/greybus/fw-management.c:9:
In function 'strncpy',
inlined from 'fw_mgmt_backend_fw_update_operation' at drivers/staging/greybus/fw-management.c:306:2:
include/linux/fortify-string.h:108:30: error: '__builtin_strncpy' specified bound 10 equals destination size [-Werror=stringop-truncation]
108 | #define __underlying_strncpy __builtin_strncpy
| ^
include/linux/fortify-string.h:187:9: note: in expansion of macro '__underlying_strncpy'
187 | return __underlying_strncpy(p, q, size);
| ^~~~~~~~~~~~~~~~~~~~
For some reason, I cannot reproduce this with gcc-9 or gcc-11, and I only
get a warning for one of the four related strncpy()s, so I'm not
sure what's going on.
Change all four to strscpy_pad(), which is the safest replacement here,
as it avoids ending up with uninitialized stack data in the tag name.
Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
---
v2
- use strscpy_pad()
- use two-argument form
- change all four instances, not just the one that produced the warning
---
drivers/staging/greybus/fw-management.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 3054f084d777..a47385175582 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -123,8 +123,7 @@ static int fw_mgmt_interface_fw_version_operation(struct fw_mgmt *fw_mgmt,
fw_info->major = le16_to_cpu(response.major);
fw_info->minor = le16_to_cpu(response.minor);
- strncpy(fw_info->firmware_tag, response.firmware_tag,
- GB_FIRMWARE_TAG_MAX_SIZE);
+ strscpy_pad(fw_info->firmware_tag, response.firmware_tag);
/*
* The firmware-tag should be NULL terminated, otherwise throw error but
@@ -153,7 +152,7 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
}
request.load_method = load_method;
- strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
+ strscpy_pad(request.firmware_tag, tag);
/*
* The firmware-tag should be NULL terminated, otherwise throw error and
@@ -249,8 +248,7 @@ static int fw_mgmt_backend_fw_version_operation(struct fw_mgmt *fw_mgmt,
struct gb_fw_mgmt_backend_fw_version_response response;
int ret;
- strncpy(request.firmware_tag, fw_info->firmware_tag,
- GB_FIRMWARE_TAG_MAX_SIZE);
+ strscpy_pad(request.firmware_tag, fw_info->firmware_tag);
/*
* The firmware-tag should be NULL terminated, otherwise throw error and
@@ -303,13 +301,13 @@ static int fw_mgmt_backend_fw_update_operation(struct fw_mgmt *fw_mgmt,
struct gb_fw_mgmt_backend_fw_update_request request;
int ret;
- strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
+ ret = strscpy_pad(request.firmware_tag, tag);
/*
* The firmware-tag should be NULL terminated, otherwise throw error and
* fail.
*/
- if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
+ if (ret == -E2BIG) {
dev_err(fw_mgmt->parent, "backend-update: firmware-tag is not NULL terminated\n");
return -EINVAL;
}
--
2.39.2