Skip to content

Commit

Permalink
drivers: fuel_gauge: kernel drivers use syscalls
Browse files Browse the repository at this point in the history
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 <aaronmassey@google.com>
  • Loading branch information
aaronemassey authored and carlescufi committed Feb 22, 2023
1 parent 7d83bf7 commit 6cdef08
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 37 deletions.
2 changes: 2 additions & 0 deletions drivers/fuel_gauge/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
48 changes: 48 additions & 0 deletions drivers/fuel_gauge/fuel_gauge_syscall_handlers.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2023 Google LLC
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/syscall_handler.h>
#include <zephyr/drivers/fuel_gauge.h>

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 <syscalls/fuel_gauge_get_prop_mrsh.c>

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 <syscalls/fuel_gauge_set_prop_mrsh.c>
65 changes: 56 additions & 9 deletions include/zephyr/drivers/fuel_gauge.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
extern "C" {
#endif /* __cplusplus */

#include <errno.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
Expand Down Expand Up @@ -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
*
Expand All @@ -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
Expand All @@ -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);
}

/**
* @}
Expand All @@ -206,4 +251,6 @@ __subsystem struct fuel_gauge_driver_api {
}
#endif /* __cplusplus */

#include <syscalls/fuel_gauge.h>

#endif /* ZEPHYR_INCLUDE_DRIVERS_BATTERY_H_ */
7 changes: 7 additions & 0 deletions tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.conf
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions tests/drivers/fuel_gauge/sbs_gauge/boards/qemu_arc_hs.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) 2023 Google LLC
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/dt-bindings/i2c/i2c.h>

/ {
/* 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 = <I2C_BITRATE_STANDARD>;
#address-cells = <1>;
#size-cells = <0>;
reg = <0x100 4>;
};
};

&fake_i2c_bus {
clock-frequency = <I2C_BITRATE_STANDARD>;
compatible = "zephyr,i2c-emul-controller";
smartbattery0: smartbattery@b {
compatible = "sbs,sbs-gauge-new-api";
reg = <0x0B>;
status = "okay";
};
};
44 changes: 18 additions & 26 deletions tests/drivers/fuel_gauge/sbs_gauge/src/test_sbs_gauge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
{
Expand All @@ -48,15 +40,15 @@ 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);

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[] = {
{
Expand All @@ -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);
Expand All @@ -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[] = {
{
Expand All @@ -97,15 +89,15 @@ 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);

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[] = {
{
Expand All @@ -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);
Expand All @@ -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 = {
Expand All @@ -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 */

Expand Down Expand Up @@ -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,
Expand All @@ -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 */

Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/drivers/fuel_gauge/sbs_gauge/testcase.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.conf
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions tests/drivers/sensor/sbs_gauge/boards/qemu_arc_hs.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) 2023 Google LLC
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/dt-bindings/i2c/i2c.h>

/ {
/* 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 = <I2C_BITRATE_STANDARD>;
#address-cells = <1>;
#size-cells = <0>;
reg = <0x100 4>;
};
};

&fake_i2c_bus {
clock-frequency = <I2C_BITRATE_STANDARD>;
compatible = "zephyr,i2c-emul-controller";
smartbattery0: smartbattery@b {
compatible = "sbs,sbs-gauge";
reg = <0x0B>;
status = "okay";
};
};
2 changes: 1 addition & 1 deletion tests/drivers/sensor/sbs_gauge/testcase.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 6cdef08

Please sign in to comment.