From 2b5e38b40c690d02e18890027b0b5abaed3918cd Mon Sep 17 00:00:00 2001 From: Brendan Le Foll Date: Fri, 3 Oct 2014 22:50:18 +0100 Subject: [PATCH] coverity: Fix issues found by coverity scan * Fix a few resource leaks in error conditions * Makes strtol() calls safer in pwm module * Make sure buffer is terminated after read() in aio Signed-off-by: Brendan Le Foll --- src/aio/aio.c | 6 ++++-- src/gpio/gpio.c | 7 +++++++ src/i2c/i2c.c | 7 +++---- src/intel_edison_fab_c.c | 2 ++ src/intel_galileo_rev_g.c | 3 ++- src/pwm/pwm.c | 28 ++++++++++++++++++++++++---- 6 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/aio/aio.c b/src/aio/aio.c index 8613352..b9989b8 100644 --- a/src/aio/aio.c +++ b/src/aio/aio.c @@ -114,7 +114,7 @@ mraa_aio_init(unsigned int aio_channel) unsigned int mraa_aio_read(mraa_aio_context dev) { - char buffer[16]; + char buffer[17]; unsigned int shifter_value = 0; if (dev->adc_in_fp == -1) { @@ -125,13 +125,15 @@ mraa_aio_read(mraa_aio_context dev) if (read(dev->adc_in_fp, buffer, sizeof(buffer)) < 1) { syslog(LOG_ERR, "Failed to read a sensible value"); } + // force NULL termination of string + buffer[16] = '\0'; lseek(dev->adc_in_fp, 0, SEEK_SET); errno = 0; char *end; unsigned int analog_value = (unsigned int) strtoul(buffer, &end, 10); if (end == &buffer[0]) { - syslog(LOG_ERR, "%s is not a decimal number", buffer); + syslog(LOG_ERR, "value is not a decimal number"); } else if (errno != 0) { syslog(LOG_ERR, "errno was set"); diff --git a/src/gpio/gpio.c b/src/gpio/gpio.c index 2bfa7dc..cf567a5 100644 --- a/src/gpio/gpio.c +++ b/src/gpio/gpio.c @@ -87,6 +87,11 @@ mraa_gpio_init_raw(int pin) int length; mraa_gpio_context dev = (mraa_gpio_context) malloc(sizeof(struct _gpio)); + if (dev == NULL) { + syslog(LOG_CRIT, "Failed to allocate memory for context"); + return NULL; + } + memset(dev, 0, sizeof(struct _gpio)); dev->value_fp = -1; dev->isr_value_fp = -1; @@ -102,6 +107,7 @@ mraa_gpio_init_raw(int pin) int export = open(SYSFS_CLASS_GPIO "/export", O_WRONLY); if (export == -1) { syslog(LOG_ERR, "Failed to open export for writing"); + free(dev); return NULL; } length = snprintf(bu, sizeof(bu), "%d", dev->pin); @@ -556,6 +562,7 @@ mraa_gpio_use_mmaped(mraa_gpio_context dev, mraa_boolean_t mmap_en) fd = open(mmp->mem_dev, O_RDWR); if (fd < 1) { syslog(LOG_ERR, "Unable to open memory device"); + close(fd); return MRAA_ERROR_INVALID_RESOURCE; } dev->reg_sz = mmp->mem_sz; diff --git a/src/i2c/i2c.c b/src/i2c/i2c.c index c53aeda..a4aa9e0 100644 --- a/src/i2c/i2c.c +++ b/src/i2c/i2c.c @@ -55,8 +55,10 @@ mraa_i2c_init_raw(unsigned int bus) return NULL; } mraa_i2c_context dev = (mraa_i2c_context) malloc(sizeof(struct _i2c)); - if (dev == NULL) + if (dev == NULL) { + syslog(LOG_CRIT, "Failed to allocate memory for context"); return NULL; + } char filepath[32]; snprintf(filepath, 32, "/dev/i2c-%u", bus); @@ -96,9 +98,6 @@ uint8_t mraa_i2c_read_byte(mraa_i2c_context dev) { uint8_t byte = i2c_smbus_read_byte(dev->fh); - if (byte < 0) { - return -1; - } return byte; } diff --git a/src/intel_edison_fab_c.c b/src/intel_edison_fab_c.c index bdecb2c..70ad5fd 100644 --- a/src/intel_edison_fab_c.c +++ b/src/intel_edison_fab_c.c @@ -69,6 +69,7 @@ mraa_intel_edison_pinmode_change(int sysfs, int mode) char mode_buf[MAX_MODE_SIZE]; int length = sprintf(mode_buf, "mode%u",mode); if (write(modef, mode_buf, length*sizeof(char)) == -1) { + close(modef); return MRAA_ERROR_INVALID_RESOURCE; } close(modef); @@ -404,6 +405,7 @@ mraa_intel_edison_fab_c() if (tristate == NULL) { syslog(LOG_CRIT, "Intel Edison Failed to initialise Arduino board TriState,\ check i2c devices! FATAL\n"); + free(b); return NULL; } mraa_gpio_dir(tristate, MRAA_GPIO_OUT); diff --git a/src/intel_galileo_rev_g.c b/src/intel_galileo_rev_g.c index 1f8322c..2a4f60b 100644 --- a/src/intel_galileo_rev_g.c +++ b/src/intel_galileo_rev_g.c @@ -153,9 +153,10 @@ mraa_intel_galileo_gen2_gpio_mode_replace(mraa_gpio_context dev, gpio_mode_t mod } if (value != -1) { sta = mraa_gpio_dir(pullup_e, MRAA_GPIO_OUT); - sta = mraa_gpio_write(pullup_e, value); + sta += mraa_gpio_write(pullup_e, value); if (sta != MRAA_SUCCESS) { syslog(LOG_ERR, "Galileo Gen 2: Error Setting pullup"); + close(drive); return sta; } } diff --git a/src/pwm/pwm.c b/src/pwm/pwm.c index 13dc493..98c241e 100644 --- a/src/pwm/pwm.c +++ b/src/pwm/pwm.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "pwm.h" #include "mraa_internal.h" @@ -100,9 +101,18 @@ mraa_pwm_read_period(mraa_pwm_context dev) read(period_f, output, size + 1); close(period_f); - int ret = strtol(output, NULL, 10); - return ret; + char *endptr; + long int ret = strtol(output, &endptr, 10); + if ('\0' != *endptr) { + syslog(LOG_ERR, "error in string converstion"); + return -1; + } + else if (ret > INT_MAX || ret < INT_MIN) { + syslog(LOG_ERR, "number is invalid"); + return -1; + } + return (int) ret; } static int @@ -118,8 +128,17 @@ mraa_pwm_read_duty(mraa_pwm_context dev) char output[MAX_SIZE]; read(dev->duty_fp, output, size+1); - int ret = strtol(output, NULL, 10); - return ret; + char *endptr; + long int ret = strtol(output, &endptr, 10); + if ('\0' != *endptr) { + syslog(LOG_ERR, "error in string converstion"); + return -1; + } + else if (ret > INT_MAX || ret < INT_MIN) { + syslog(LOG_ERR, "number is invalid"); + return -1; + } + return (int) ret; } mraa_pwm_context @@ -178,6 +197,7 @@ mraa_pwm_init_raw(int chipin, int pin) if (write(export_f, out, size*sizeof(char)) == -1) { syslog(LOG_WARNING, "Failed to write to export! Potentially already enabled"); close(export_f); + free(dev); return NULL; } dev->owner = 1;