From 6cdef0894fe7aced7f58b1f34fa1ca2b691f56cb Mon Sep 17 00:00:00 2001 From: Aaron Massey Date: Thu, 29 Dec 2022 15:05:40 -0700 Subject: [PATCH] drivers: fuel_gauge: kernel drivers use syscalls The fuel gauge API doesn't make use of syscalls with device drivers, which is not congruent with Zephyr's security model. Correct the fuel gauge API to only allow access to fuel gauge device drivers functionality via syscalls to keep with Zephyr's security model. Signed-off-by: Aaron Massey --- drivers/fuel_gauge/CMakeLists.txt | 2 + .../fuel_gauge/fuel_gauge_syscall_handlers.c | 48 ++++++++++++++ include/zephyr/drivers/fuel_gauge.h | 65 ++++++++++++++++--- .../sbs_gauge/boards/qemu_arc_hs.conf | 7 ++ .../sbs_gauge/boards/qemu_arc_hs.overlay | 29 +++++++++ .../fuel_gauge/sbs_gauge/src/test_sbs_gauge.c | 44 +++++-------- .../fuel_gauge/sbs_gauge/testcase.yaml | 2 +- .../sensor/sbs_gauge/boards/qemu_arc_hs.conf | 7 ++ .../sbs_gauge/boards/qemu_arc_hs.overlay | 29 +++++++++ tests/drivers/sensor/sbs_gauge/testcase.yaml | 2 +- 10 files changed, 198 insertions(+), 37 deletions(-) create mode 100644 drivers/fuel_gauge/fuel_gauge_syscall_handlers.c create mode 100644 tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.conf create mode 100644 tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.overlay create mode 100644 tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.conf create mode 100644 tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.overlay diff --git a/drivers/fuel_gauge/CMakeLists.txt b/drivers/fuel_gauge/CMakeLists.txt index c68e640c34d943..aa58fc36a926b0 100644 --- a/drivers/fuel_gauge/CMakeLists.txt +++ b/drivers/fuel_gauge/CMakeLists.txt @@ -1,3 +1,5 @@ # SPDX-License-Identifier: Apache-2.0 add_subdirectory_ifdef(CONFIG_SBS_GAUGE_NEW_API sbs_gauge) + +zephyr_library_sources_ifdef(CONFIG_USERSPACE fuel_gauge_syscall_handlers.c) diff --git a/drivers/fuel_gauge/fuel_gauge_syscall_handlers.c b/drivers/fuel_gauge/fuel_gauge_syscall_handlers.c new file mode 100644 index 00000000000000..ef3c8f5dc305b3 --- /dev/null +++ b/drivers/fuel_gauge/fuel_gauge_syscall_handlers.c @@ -0,0 +1,48 @@ +/* + * Copyright 2023 Google LLC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +static inline int z_vrfy_fuel_gauge_get_prop(const struct device *dev, + struct fuel_gauge_get_property *props, + size_t props_len) +{ + struct fuel_gauge_get_property k_props[props_len]; + + Z_OOPS(Z_SYSCALL_DRIVER_FUEL_GAUGE(dev, get_property)); + + Z_OOPS(z_user_from_copy(k_props, props, + props_len * sizeof(struct fuel_gauge_get_property))); + + int ret = z_impl_fuel_gauge_get_prop(dev, k_props, props_len); + + Z_OOPS(z_user_to_copy(props, k_props, props_len * sizeof(struct fuel_gauge_get_property))); + + return ret; +} + +#include + +static inline int z_vrfy_fuel_gauge_set_prop(const struct device *dev, + struct fuel_gauge_set_property *props, + size_t props_len) +{ + struct fuel_gauge_set_property k_props[props_len]; + + Z_OOPS(Z_SYSCALL_DRIVER_FUEL_GAUGE(dev, set_property)); + + Z_OOPS(z_user_from_copy(k_props, props, + props_len * sizeof(struct fuel_gauge_set_property))); + + int ret = z_impl_fuel_gauge_set_prop(dev, k_props, props_len); + + Z_OOPS(z_user_to_copy(props, k_props, props_len * sizeof(struct fuel_gauge_set_property))); + + return ret; +} + +#include diff --git a/include/zephyr/drivers/fuel_gauge.h b/include/zephyr/drivers/fuel_gauge.h index 79655449e26b66..577593a1950242 100644 --- a/include/zephyr/drivers/fuel_gauge.h +++ b/include/zephyr/drivers/fuel_gauge.h @@ -18,6 +18,7 @@ extern "C" { #endif /* __cplusplus */ +#include #include #include #include @@ -161,6 +162,31 @@ struct fuel_gauge_set_property { } value; }; +/** + * @typedef fuel_gauge_get_property_t + * @brief Callback API for getting a fuel_gauge property. + * + * See fuel_gauge_get_property() for argument description + */ +typedef int (*fuel_gauge_get_property_t)(const struct device *dev, + struct fuel_gauge_get_property *props, size_t props_len); + +/** + * @typedef fuel_gauge_set_property_t + * @brief Callback API for setting a fuel_gauge property. + * + * See fuel_gauge_set_property() for argument description + */ +typedef int (*fuel_gauge_set_property_t)(const struct device *dev, + struct fuel_gauge_set_property *props, size_t props_len); + +/* Caching is entirely on the onus of the client */ + +__subsystem struct fuel_gauge_driver_api { + fuel_gauge_get_property_t get_property; + fuel_gauge_set_property_t set_property; +}; + /** * @brief Fetch a battery fuel-gauge property * @@ -173,8 +199,21 @@ struct fuel_gauge_set_property { * @return return=0 if successful, return < 0 if getting all properties failed, return > 0 if some * properties failed where return=number of failing properties. */ -typedef int (*fuel_gauge_get_property_t)(const struct device *dev, - struct fuel_gauge_get_property *props, size_t props_len); +__syscall int fuel_gauge_get_prop(const struct device *dev, struct fuel_gauge_get_property *props, + size_t props_len); + +static inline int z_impl_fuel_gauge_get_prop(const struct device *dev, + struct fuel_gauge_get_property *props, + size_t props_len) +{ + const struct fuel_gauge_driver_api *api = (const struct fuel_gauge_driver_api *)dev->api; + + if (api->get_property == NULL) { + return -ENOSYS; + } + + return api->get_property(dev, props, props_len); +} /** * @brief Set a battery fuel-gauge property @@ -188,15 +227,21 @@ typedef int (*fuel_gauge_get_property_t)(const struct device *dev, * @return return=0 if successful, return < 0 if setting all properties failed, return > 0 if some * properties failed where return=number of failing properties. */ -typedef int (*fuel_gauge_set_property_t)(const struct device *dev, - struct fuel_gauge_set_property *props, size_t props_len); +__syscall int fuel_gauge_set_prop(const struct device *dev, struct fuel_gauge_set_property *props, + size_t props_len); -/* Caching is entirely on the onus of the client */ +static inline int z_impl_fuel_gauge_set_prop(const struct device *dev, + struct fuel_gauge_set_property *props, + size_t props_len) +{ + const struct fuel_gauge_driver_api *api = (const struct fuel_gauge_driver_api *)dev->api; -__subsystem struct fuel_gauge_driver_api { - fuel_gauge_get_property_t get_property; - fuel_gauge_set_property_t set_property; -}; + if (api->set_property == NULL) { + return -ENOSYS; + } + + return api->set_property(dev, props, props_len); +} /** * @} @@ -206,4 +251,6 @@ __subsystem struct fuel_gauge_driver_api { } #endif /* __cplusplus */ +#include + #endif /* ZEPHYR_INCLUDE_DRIVERS_BATTERY_H_ */ diff --git a/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.conf b/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.conf new file mode 100644 index 00000000000000..1ddb17feec59ef --- /dev/null +++ b/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.conf @@ -0,0 +1,7 @@ +# Copyright (c) 2023 Google LLC +# SPDX-License-Identifier: Apache-2.0 + +CONFIG_EMUL=y +CONFIG_EMUL_SBS_GAUGE=y +CONFIG_I2C=y +CONFIG_I2C_EMUL=y diff --git a/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.overlay b/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.overlay new file mode 100644 index 00000000000000..7411c808b8b62c --- /dev/null +++ b/tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.overlay @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2023 Google LLC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +/ { + /* qemu_cortex_a9 board isn't configured with an I2C node */ + fake_i2c_bus: i2c@100 { + status = "okay"; + compatible = "zephyr,i2c-emul-controller"; + clock-frequency = ; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x100 4>; + }; +}; + +&fake_i2c_bus { + clock-frequency = ; + compatible = "zephyr,i2c-emul-controller"; + smartbattery0: smartbattery@b { + compatible = "sbs,sbs-gauge-new-api"; + reg = <0x0B>; + status = "okay"; + }; +}; diff --git a/tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c b/tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c index 04c19aab424e8e..15bcd6ed0d7235 100644 --- a/tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c +++ b/tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c @@ -20,26 +20,18 @@ struct sbs_gauge_new_api_fixture { static void *sbs_gauge_new_api_setup(void) { + static ZTEST_DMEM struct sbs_gauge_new_api_fixture fixture; - static struct sbs_gauge_new_api_fixture fixture; - const struct device *dev = DEVICE_DT_GET_ANY(sbs_sbs_gauge_new_api); - const struct fuel_gauge_driver_api *api = dev->api; + fixture.dev = DEVICE_DT_GET_ANY(sbs_sbs_gauge_new_api); - fixture.dev = dev; - fixture.api = api; + k_object_access_all_grant(fixture.dev); zassert_true(device_is_ready(fixture.dev), "Fuel Gauge not found"); - zassert_not_null(api); return &fixture; } -ZTEST_F(sbs_gauge_new_api, test_implemented_apis) -{ - zassert_not_equal(NULL, fixture->api->get_property); -} - -ZTEST_F(sbs_gauge_new_api, test_get_all_props_failed_returns_negative) +ZTEST_USER_F(sbs_gauge_new_api, test_get_all_props_failed_returns_negative) { struct fuel_gauge_get_property props[] = { { @@ -48,7 +40,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_all_props_failed_returns_negative) }, }; - int ret = fixture->api->get_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_get_prop(fixture->dev, props, ARRAY_SIZE(props)); zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.", props[0].property_type); @@ -56,7 +48,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_all_props_failed_returns_negative) zassert_true(ret < 0); } -ZTEST_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_count) +ZTEST_USER_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_count) { struct fuel_gauge_get_property props[] = { { @@ -74,7 +66,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_count) }; - int ret = fixture->api->get_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_get_prop(fixture->dev, props, ARRAY_SIZE(props)); zassert_equal(props[0].status, -ENOTSUP, "Getting bad property %d has a good status.", props[0].property_type); @@ -88,7 +80,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_some_props_failed_returns_failed_prop_count) zassert_equal(ret, 2); } -ZTEST_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative) +ZTEST_USER_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative) { struct fuel_gauge_set_property props[] = { { @@ -97,7 +89,7 @@ ZTEST_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative) }, }; - int ret = fixture->api->set_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_set_prop(fixture->dev, props, ARRAY_SIZE(props)); zassert_equal(props[0].status, -ENOTSUP, "Setting bad property %d has a good status.", props[0].property_type); @@ -105,7 +97,7 @@ ZTEST_F(sbs_gauge_new_api, test_set_all_props_failed_returns_negative) zassert_true(ret < 0); } -ZTEST_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_count) +ZTEST_USER_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_count) { struct fuel_gauge_set_property props[] = { { @@ -125,7 +117,7 @@ ZTEST_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_count) }; - int ret = fixture->api->set_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_set_prop(fixture->dev, props, ARRAY_SIZE(props)); zassert_equal(props[0].status, -ENOTSUP, "Setting bad property %d has a good status.", props[0].property_type); @@ -139,7 +131,7 @@ ZTEST_F(sbs_gauge_new_api, test_set_some_props_failed_returns_failed_prop_count) zassert_equal(ret, 2); } -ZTEST_F(sbs_gauge_new_api, test_set_prop_can_be_get) +ZTEST_USER_F(sbs_gauge_new_api, test_set_prop_can_be_get) { uint16_t word = BIT(15) | BIT(0); struct fuel_gauge_set_property set_prop = { @@ -149,19 +141,19 @@ ZTEST_F(sbs_gauge_new_api, test_set_prop_can_be_get) .value.sbs_mfr_access_word = word, }; - zassert_ok(fixture->api->set_property(fixture->dev, &set_prop, 1)); + zassert_ok(fuel_gauge_set_prop(fixture->dev, &set_prop, 1)); zassert_ok(set_prop.status); struct fuel_gauge_get_property get_prop = { .property_type = FUEL_GAUGE_SBS_MFR_ACCESS, }; - zassert_ok(fixture->api->get_property(fixture->dev, &get_prop, 1)); + zassert_ok(fuel_gauge_get_prop(fixture->dev, &get_prop, 1)); zassert_ok(get_prop.status); zassert_equal(get_prop.value.sbs_mfr_access_word, word); } -ZTEST_F(sbs_gauge_new_api, test_get_props__returns_ok) +ZTEST_USER_F(sbs_gauge_new_api, test_get_props__returns_ok) { /* Validate what props are supported by the driver */ @@ -219,7 +211,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_props__returns_ok) }, }; - int ret = fixture->api->get_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_get_prop(fixture->dev, props, ARRAY_SIZE(props)); for (int i = 0; i < ARRAY_SIZE(props); i++) { zassert_ok(props[i].status, "Property %d getting %d has a bad status.", i, @@ -229,7 +221,7 @@ ZTEST_F(sbs_gauge_new_api, test_get_props__returns_ok) zassert_ok(ret); } -ZTEST_F(sbs_gauge_new_api, test_set_props__returns_ok) +ZTEST_USER_F(sbs_gauge_new_api, test_set_props__returns_ok) { /* Validate what props are supported by the driver */ @@ -239,7 +231,7 @@ ZTEST_F(sbs_gauge_new_api, test_set_props__returns_ok) }, }; - int ret = fixture->api->set_property(fixture->dev, props, ARRAY_SIZE(props)); + int ret = fuel_gauge_set_prop(fixture->dev, props, ARRAY_SIZE(props)); for (int i = 0; i < ARRAY_SIZE(props); i++) { zassert_ok(props[i].status, "Property %d writing %d has a bad status.", i, diff --git a/tests/drivers/fuel_gauge/sbs_gauge/testcase.yaml b/tests/drivers/fuel_gauge/sbs_gauge/testcase.yaml index 5e0dda2370e5d2..ab511a255e5b57 100644 --- a/tests/drivers/fuel_gauge/sbs_gauge/testcase.yaml +++ b/tests/drivers/fuel_gauge/sbs_gauge/testcase.yaml @@ -3,6 +3,6 @@ tests: drivers.sbs_gauge_new_api: tags: test_framework filter: dt_compat_enabled("sbs,sbs-gauge-new-api") - platform_allow: native_posix qemu_cortex_a9 nucleo_f070rb + platform_allow: native_posix qemu_cortex_a9 nucleo_f070rb qemu_arc_hs integration_platforms: - nucleo_f070rb diff --git a/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.conf b/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.conf new file mode 100644 index 00000000000000..1ddb17feec59ef --- /dev/null +++ b/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.conf @@ -0,0 +1,7 @@ +# Copyright (c) 2023 Google LLC +# SPDX-License-Identifier: Apache-2.0 + +CONFIG_EMUL=y +CONFIG_EMUL_SBS_GAUGE=y +CONFIG_I2C=y +CONFIG_I2C_EMUL=y diff --git a/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.overlay b/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.overlay new file mode 100644 index 00000000000000..499a716892b86f --- /dev/null +++ b/tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.overlay @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2023 Google LLC + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +/ { + /* qemu_cortex_a9 board isn't configured with an I2C node */ + fake_i2c_bus: i2c@100 { + status = "okay"; + compatible = "zephyr,i2c-emul-controller"; + clock-frequency = ; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x100 4>; + }; +}; + +&fake_i2c_bus { + clock-frequency = ; + compatible = "zephyr,i2c-emul-controller"; + smartbattery0: smartbattery@b { + compatible = "sbs,sbs-gauge"; + reg = <0x0B>; + status = "okay"; + }; +}; diff --git a/tests/drivers/sensor/sbs_gauge/testcase.yaml b/tests/drivers/sensor/sbs_gauge/testcase.yaml index 2f7d4285041329..61dfc364c9686b 100644 --- a/tests/drivers/sensor/sbs_gauge/testcase.yaml +++ b/tests/drivers/sensor/sbs_gauge/testcase.yaml @@ -9,6 +9,6 @@ tests: drivers.sbs_gauge.emulated: tags: test_framework filter: dt_compat_enabled("sbs,sbs-gauge") - platform_allow: native_posix qemu_cortex_a9 + platform_allow: native_posix qemu_cortex_a9 qemu_arc_hs integration_platforms: - native_posix