Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: counter: Fix possible out-of-bounds access in MCP7940N driver #84533

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

drivers: counter: Fix possible out-of-bounds access in MCP7940N driver #84533

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 48 additions & 68 deletions drivers/counter/rtc_mcp7940n.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
* Copyright (c) 2019-2020 Peter Bigot Consulting, LLC
* Copyright (c) 2021 Laird Connectivity
* Copyright (c) 2025 Marcin Lyda <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -26,20 +27,17 @@
LOG_MODULE_REGISTER(MCP7940N, CONFIG_COUNTER_LOG_LEVEL);

/* Alarm channels */
#define ALARM0_ID 0
#define ALARM1_ID 1
#define ALARM0_ID 0
#define ALARM1_ID 1

/* Size of block when writing whole struct */
#define RTC_TIME_REGISTERS_SIZE sizeof(struct mcp7940n_time_registers)
#define RTC_ALARM_REGISTERS_SIZE sizeof(struct mcp7940n_alarm_registers)

/* Largest block size */
#define MAX_WRITE_SIZE (RTC_TIME_REGISTERS_SIZE)
#define RTC_TIME_REGISTERS_SIZE sizeof(struct mcp7940n_time_registers)
#define RTC_ALARM_REGISTERS_SIZE sizeof(struct mcp7940n_alarm_registers)

/* tm struct uses years since 1900 but unix time uses years since
* 1970. MCP7940N default year is '1' so the offset is 69
*/
#define UNIX_YEAR_OFFSET 69
#define UNIX_YEAR_OFFSET 69

/* Macro used to decode BCD to UNIX time to avoid potential copy and paste
* errors.
Expand Down Expand Up @@ -79,7 +77,7 @@ static time_t decode_rtc(const struct device *dev)
{
struct mcp7940n_data *data = dev->data;
time_t time_unix = 0;
struct tm time = { 0 };
struct tm time = {0};

time.tm_sec = RTC_BCD_DECODE(data->registers.rtc_sec.sec);
time.tm_min = RTC_BCD_DECODE(data->registers.rtc_min.min);
Expand All @@ -89,8 +87,7 @@ static time_t decode_rtc(const struct device *dev)
/* tm struct starts months at 0, mcp7940n starts at 1 */
time.tm_mon = RTC_BCD_DECODE(data->registers.rtc_month.month) - 1;
/* tm struct uses years since 1900 but unix time uses years since 1970 */
time.tm_year = RTC_BCD_DECODE(data->registers.rtc_year.year) +
UNIX_YEAR_OFFSET;
time.tm_year = RTC_BCD_DECODE(data->registers.rtc_year.year) + UNIX_YEAR_OFFSET;

time_unix = timeutil_timegm(&time);

Expand Down Expand Up @@ -252,43 +249,41 @@ static int write_register(const struct device *dev, enum mcp7940n_register addr,
*
* @param dev the MCP7940N device pointer.
* @param addr first register address to write to, should be REG_RTC_SEC,
* REG_ALM0_SEC or REG_ALM0_SEC.
* @param size size of data struct that will be written.
* REG_ALM0_SEC or REG_ALM1_SEC.
*
* @retval return 0 on success, or a negative error code from an I2C
* transaction or invalid parameter.
*/
static int write_data_block(const struct device *dev, enum mcp7940n_register addr, uint8_t size)
static int write_data_block(const struct device *dev, enum mcp7940n_register addr)
{
struct mcp7940n_data *data = dev->data;
const struct mcp7940n_config *cfg = dev->config;
int rc = 0;
uint8_t time_data[MAX_WRITE_SIZE + 1];
uint8_t *write_block_start;

if (size > MAX_WRITE_SIZE) {
return -EINVAL;
}
int rc;
const uint8_t *write_block_start;
uint8_t write_block_size;

if (addr >= REG_INVAL) {
return -EINVAL;
}

if (addr == REG_RTC_SEC) {
switch (addr) {
case REG_RTC_SEC:
write_block_start = (uint8_t *)&data->registers;
} else if (addr == REG_ALM0_SEC) {
write_block_size = sizeof(data->registers);
break;
case REG_ALM0_SEC:
write_block_start = (uint8_t *)&data->alm0_registers;
} else if (addr == REG_ALM1_SEC) {
write_block_size = sizeof(data->alm0_registers);
break;
case REG_ALM1_SEC:
write_block_start = (uint8_t *)&data->alm1_registers;
} else {
write_block_size = sizeof(data->alm1_registers);
break;
default:
return -EINVAL;
}

/* Load register address into first byte then fill in data values */
time_data[0] = addr;
memcpy(&time_data[1], write_block_start, size);

rc = i2c_write_dt(&cfg->i2c, time_data, size + 1);
rc = i2c_burst_write_dt(&cfg->i2c, addr, write_block_start, write_block_size);

return rc;
}
Expand All @@ -308,13 +303,13 @@ static int write_data_block(const struct device *dev, enum mcp7940n_register add
static int set_day_of_week(const struct device *dev, time_t *unix_time)
{
struct mcp7940n_data *data = dev->data;
struct tm time_buffer = { 0 };
struct tm time_buffer = {0};
int rc = 0;

if (gmtime_r(unix_time, &time_buffer) != NULL) {
data->registers.rtc_weekday.weekday = time_buffer.tm_wday;
rc = write_register(dev, REG_RTC_WDAY,
*((uint8_t *)(&data->registers.rtc_weekday)));
*((uint8_t *)(&data->registers.rtc_weekday)));
} else {
rc = -EINVAL;
}
Expand Down Expand Up @@ -356,8 +351,7 @@ static void mcp7940n_handle_interrupt(const struct device *dev, uint8_t alarm_id
if (alm_regs->alm_weekday.alm_if) {
/* Clear interrupt */
alm_regs->alm_weekday.alm_if = 0;
write_register(dev, alarm_reg_address,
*((uint8_t *)(&alm_regs->alm_weekday)));
write_register(dev, alarm_reg_address, *((uint8_t *)(&alm_regs->alm_weekday)));

/* Fire callback */
if (data->counter_handler[alarm_id]) {
Expand All @@ -376,19 +370,16 @@ static void mcp7940n_handle_interrupt(const struct device *dev, uint8_t alarm_id

static void mcp7940n_work_handler(struct k_work *work)
{
struct mcp7940n_data *data =
CONTAINER_OF(work, struct mcp7940n_data, alarm_work);
struct mcp7940n_data *data = CONTAINER_OF(work, struct mcp7940n_data, alarm_work);

/* Check interrupt flags for both alarms */
mcp7940n_handle_interrupt(data->mcp7940n, ALARM0_ID);
mcp7940n_handle_interrupt(data->mcp7940n, ALARM1_ID);
}

static void mcp7940n_init_cb(const struct device *dev,
struct gpio_callback *gpio_cb, uint32_t pins)
static void mcp7940n_init_cb(const struct device *dev, struct gpio_callback *gpio_cb, uint32_t pins)
{
struct mcp7940n_data *data =
CONTAINER_OF(gpio_cb, struct mcp7940n_data, int_callback);
struct mcp7940n_data *data = CONTAINER_OF(gpio_cb, struct mcp7940n_data, int_callback);

ARG_UNUSED(pins);

Expand All @@ -398,7 +389,7 @@ static void mcp7940n_init_cb(const struct device *dev,
int mcp7940n_rtc_set_time(const struct device *dev, time_t unix_time)
{
struct mcp7940n_data *data = dev->data;
struct tm time_buffer = { 0 };
struct tm time_buffer = {0};
int rc = 0;

if (unix_time > UINT32_MAX) {
Expand All @@ -421,7 +412,7 @@ int mcp7940n_rtc_set_time(const struct device *dev, time_t unix_time)
}

/* Write to device */
rc = write_data_block(dev, REG_RTC_SEC, RTC_TIME_REGISTERS_SIZE);
rc = write_data_block(dev, REG_RTC_SEC);

out:
k_sem_give(&data->lock);
Expand All @@ -438,8 +429,7 @@ static int mcp7940n_counter_start(const struct device *dev)

/* Set start oscillator configuration bit */
data->registers.rtc_sec.start_osc = 1;
rc = write_register(dev, REG_RTC_SEC,
*((uint8_t *)(&data->registers.rtc_sec)));
rc = write_register(dev, REG_RTC_SEC, *((uint8_t *)(&data->registers.rtc_sec)));

k_sem_give(&data->lock);

Expand All @@ -455,16 +445,14 @@ static int mcp7940n_counter_stop(const struct device *dev)

/* Clear start oscillator configuration bit */
data->registers.rtc_sec.start_osc = 0;
rc = write_register(dev, REG_RTC_SEC,
*((uint8_t *)(&data->registers.rtc_sec)));
rc = write_register(dev, REG_RTC_SEC, *((uint8_t *)(&data->registers.rtc_sec)));

k_sem_give(&data->lock);

return rc;
}

static int mcp7940n_counter_get_value(const struct device *dev,
uint32_t *ticks)
static int mcp7940n_counter_get_value(const struct device *dev, uint32_t *ticks)
{
struct mcp7940n_data *data = dev->data;
time_t unix_time;
Expand Down Expand Up @@ -492,7 +480,7 @@ static int mcp7940n_counter_set_alarm(const struct device *dev, uint8_t alarm_id
uint32_t seconds_until_alarm;
time_t current_time;
time_t alarm_time;
struct tm time_buffer = { 0 };
struct tm time_buffer = {0};
uint8_t alarm_base_address;
struct mcp7940n_alarm_registers *alm_regs;
int rc = 0;
Expand Down Expand Up @@ -536,14 +524,13 @@ static int mcp7940n_counter_set_alarm(const struct device *dev, uint8_t alarm_id

/* Write time to alarm registers */
encode_alarm(dev, &time_buffer, alarm_id);
rc = write_data_block(dev, alarm_base_address, RTC_ALARM_REGISTERS_SIZE);
rc = write_data_block(dev, alarm_base_address);
if (rc < 0) {
goto out;
}

/* Enable alarm */
rc = write_register(dev, REG_RTC_CONTROL,
*((uint8_t *)(&data->registers.rtc_control)));
rc = write_register(dev, REG_RTC_CONTROL, *((uint8_t *)(&data->registers.rtc_control)));
if (rc < 0) {
goto out;
}
Expand Down Expand Up @@ -576,8 +563,7 @@ static int mcp7940n_counter_cancel_alarm(const struct device *dev, uint8_t alarm
goto out;
}

rc = write_register(dev, REG_RTC_CONTROL,
*((uint8_t *)(&data->registers.rtc_control)));
rc = write_register(dev, REG_RTC_CONTROL, *((uint8_t *)(&data->registers.rtc_control)));

out:
k_sem_give(&data->lock);
Expand Down Expand Up @@ -608,8 +594,7 @@ static uint32_t mcp7940n_counter_get_pending_int(const struct device *dev)
k_sem_take(&data->lock, K_FOREVER);

/* Check interrupt flag for alarm 0 */
rc = read_register(dev, REG_ALM0_WDAY,
(uint8_t *)&data->alm0_registers.alm_weekday);
rc = read_register(dev, REG_ALM0_WDAY, (uint8_t *)&data->alm0_registers.alm_weekday);
if (rc < 0) {
goto out;
}
Expand All @@ -618,16 +603,15 @@ static uint32_t mcp7940n_counter_get_pending_int(const struct device *dev)
/* Clear interrupt */
data->alm0_registers.alm_weekday.alm_if = 0;
rc = write_register(dev, REG_ALM0_WDAY,
*((uint8_t *)(&data->alm0_registers.alm_weekday)));
*((uint8_t *)(&data->alm0_registers.alm_weekday)));
if (rc < 0) {
goto out;
}
interrupt_pending |= (1 << ALARM0_ID);
}

/* Check interrupt flag for alarm 1 */
rc = read_register(dev, REG_ALM1_WDAY,
(uint8_t *)&data->alm1_registers.alm_weekday);
rc = read_register(dev, REG_ALM1_WDAY, (uint8_t *)&data->alm1_registers.alm_weekday);
if (rc < 0) {
goto out;
}
Expand All @@ -636,7 +620,7 @@ static uint32_t mcp7940n_counter_get_pending_int(const struct device *dev)
/* Clear interrupt */
data->alm1_registers.alm_weekday.alm_if = 0;
rc = write_register(dev, REG_ALM1_WDAY,
*((uint8_t *)(&data->alm1_registers.alm_weekday)));
*((uint8_t *)(&data->alm1_registers.alm_weekday)));
if (rc < 0) {
goto out;
}
Expand Down Expand Up @@ -685,8 +669,7 @@ static int mcp7940n_init(const struct device *dev)

/* Set 24-hour time */
data->registers.rtc_hours.twelve_hr = false;
rc = write_register(dev, REG_RTC_HOUR,
*((uint8_t *)(&data->registers.rtc_hours)));
rc = write_register(dev, REG_RTC_HOUR, *((uint8_t *)(&data->registers.rtc_hours)));
if (rc < 0) {
goto out;
}
Expand All @@ -695,8 +678,7 @@ static int mcp7940n_init(const struct device *dev)
if (cfg->int_gpios.port != NULL) {

if (!gpio_is_ready_dt(&cfg->int_gpios)) {
LOG_ERR("Port device %s is not ready",
cfg->int_gpios.port->name);
LOG_ERR("Port device %s is not ready", cfg->int_gpios.port->name);
rc = -ENODEV;
goto out;
}
Expand All @@ -706,11 +688,9 @@ static int mcp7940n_init(const struct device *dev)

gpio_pin_configure_dt(&cfg->int_gpios, GPIO_INPUT);

gpio_pin_interrupt_configure_dt(&cfg->int_gpios,
GPIO_INT_EDGE_TO_ACTIVE);
gpio_pin_interrupt_configure_dt(&cfg->int_gpios, GPIO_INT_EDGE_TO_ACTIVE);

gpio_init_callback(&data->int_callback, mcp7940n_init_cb,
BIT(cfg->int_gpios.pin));
gpio_init_callback(&data->int_callback, mcp7940n_init_cb, BIT(cfg->int_gpios.pin));

(void)gpio_add_callback(cfg->int_gpios.port, &data->int_callback);

Expand Down
Loading