From 5a42f32e30608c5763da0d605f2a7e378ee0009c Mon Sep 17 00:00:00 2001 From: Oleksiy Slyshyk Date: Sun, 12 May 2019 12:59:01 +0300 Subject: [PATCH] fixed few potential memory/resource leaks also few cppcheck warnings --- src/common.c | 14 +++--- src/gdbserver/gdb-remote.c | 15 +++++- src/gdbserver/gdb-server.c | 93 ++++++++++++++++++------------------- src/gdbserver/semihosting.c | 2 +- src/mingw/mingw.c | 3 +- src/mingw/mingw.h | 2 +- src/mmap.c | 6 ++- src/sg.c | 2 +- src/usb.c | 2 +- 9 files changed, 78 insertions(+), 61 deletions(-) diff --git a/src/common.c b/src/common.c index 63de8415c..19cfd4f87 100644 --- a/src/common.c +++ b/src/common.c @@ -288,7 +288,7 @@ static inline unsigned int is_flash_locked(stlink_t *sl) { else cr_lock_shift = FLASH_CR_LOCK; - return cr & (1 << cr_lock_shift); + return cr & (1u << cr_lock_shift); } static void unlock_flash(stlink_t *sl) { @@ -352,11 +352,11 @@ static void lock_flash(stlink_t *sl) { cr_lock_shift = FLASH_CR_LOCK; } - n = read_flash_cr(sl) | (1 << cr_lock_shift); + n = read_flash_cr(sl) | (1u << cr_lock_shift); stlink_write_debug32(sl, cr_reg, n); if (sl->flash_type == STLINK_FLASH_TYPE_F1_XL) { - n = read_flash_cr2(sl) | (1 << cr_lock_shift); + n = read_flash_cr2(sl) | (1u << cr_lock_shift); stlink_write_debug32(sl, FLASH_CR2, n); } } @@ -2524,7 +2524,7 @@ int stlink_write_option_bytes(stlink_t *sl, stm32_addr_t addr, uint8_t* base, ui /* Unlock flash if necessary (ref manuel page 52) */ stlink_read_debug32(sl, STM32G0_FLASH_CR, &val); - if ((val & (1 << STM32G0_FLASH_CR_LOCK))) { + if ((val & (1u << STM32G0_FLASH_CR_LOCK))) { /* disable flash write protection. */ stlink_write_debug32(sl, STM32G0_FLASH_KEYR, 0x45670123); @@ -2532,7 +2532,7 @@ int stlink_write_option_bytes(stlink_t *sl, stm32_addr_t addr, uint8_t* base, ui // check that the lock is no longer set. stlink_read_debug32(sl, STM32G0_FLASH_CR, &val); - if ((val & (1 << STM32G0_FLASH_CR_LOCK))) { + if ((val & (1u << STM32G0_FLASH_CR_LOCK))) { ELOG("Flash unlock failed! System reset required to be able to unlock it again!\n"); return -1; } @@ -2578,11 +2578,11 @@ int stlink_write_option_bytes(stlink_t *sl, stm32_addr_t addr, uint8_t* base, ui /* Re-lock option bytes */ stlink_read_debug32(sl, STM32G0_FLASH_CR, &val); - val |= (1 << STM32G0_FLASH_CR_OPTLOCK); + val |= (1u << STM32G0_FLASH_CR_OPTLOCK); stlink_write_debug32(sl, STM32G0_FLASH_CR, val); /* Re-lock flash. */ stlink_read_debug32(sl, STM32G0_FLASH_CR, &val); - val |= (1 << STM32G0_FLASH_CR_LOCK); + val |= (1u << STM32G0_FLASH_CR_LOCK); stlink_write_debug32(sl, STM32G0_FLASH_CR, val); return 0; diff --git a/src/gdbserver/gdb-remote.c b/src/gdbserver/gdb-remote.c index 83c5e093a..e4aad312d 100644 --- a/src/gdbserver/gdb-remote.c +++ b/src/gdbserver/gdb-remote.c @@ -64,8 +64,12 @@ int gdb_recv_packet(int fd, char** buffer) { char* packet_buffer = malloc(packet_size); unsigned state; + if(packet_buffer == NULL) + return -2; + start: state = 0; + packet_idx = 0; /* * 0: waiting $ * 1: data, waiting # @@ -77,6 +81,7 @@ int gdb_recv_packet(int fd, char** buffer) { char c; while(state != 4) { if(read(fd, &c, 1) != 1) { + free(packet_buffer); return -2; } @@ -98,7 +103,13 @@ int gdb_recv_packet(int fd, char** buffer) { if(packet_idx == packet_size) { packet_size += ALLOC_STEP; - packet_buffer = realloc(packet_buffer, packet_size); + void* p = realloc(packet_buffer, packet_size); + if(p != NULL) + packet_buffer = p; + else { + free(packet_buffer); + return -2; + } } } break; @@ -119,6 +130,7 @@ int gdb_recv_packet(int fd, char** buffer) { if(recv_cksum_int != cksum) { char nack = '-'; if(write(fd, &nack, 1) != 1) { + free(packet_buffer); return -2; } @@ -126,6 +138,7 @@ int gdb_recv_packet(int fd, char** buffer) { } else { char ack = '+'; if(write(fd, &ack, 1) != 1) { + free(packet_buffer); return -2; } } diff --git a/src/gdbserver/gdb-server.c b/src/gdbserver/gdb-server.c index 8566b792e..bdfeb703d 100644 --- a/src/gdbserver/gdb-server.c +++ b/src/gdbserver/gdb-server.c @@ -14,7 +14,7 @@ #include #define __attribute__(x) #endif -#if defined(__MINGW32__) || defined(_MSC_VER) +#if defined(_WIN32) #include #else #include @@ -44,6 +44,15 @@ static bool semihosting = false; static bool serial_specified = false; static char serialnumber[28] = {0}; +#if defined(_WIN32) +#define close_socket win32_close_socket +#define IS_SOCK_VALID(__sock) ((__sock) != INVALID_SOCKET) +#else +#define close_socket close +#define SOCKET int +#define IS_SOCK_VALID(__sock) ((__sock) > 0) +#endif + static const char hex[] = "0123456789abcdef"; static const char* current_memory_map = NULL; @@ -106,7 +115,7 @@ int parse_options(int argc, char** argv, st_state_t *st) { {"no-reset", optional_argument, NULL, 'n'}, {"version", no_argument, NULL, 'V'}, {"semihosting", no_argument, NULL, SEMIHOSTING_OPTION}, - {"serial", required_argument, NULL, SERIAL_OPTION}, + {"serial", required_argument, NULL, SERIAL_OPTION}, {0, 0, 0, 0}, }; const char * help_str = "%s - usage:\n\n" @@ -863,7 +872,6 @@ static int flash_go(stlink_t *sl) { int ret = stlink_write_flash(sl, page, fb->data + (page - fb->addr), len, 0); if (ret < 0) goto error; - length -= len; } } @@ -989,29 +997,27 @@ static void init_cache (stlink_t *sl) { } static void cache_flush(stlink_t *sl, unsigned ccr) { - int level; - - if (ccr & CCR_DC) - for (level = cache_desc.louu - 1; level >= 0; level--) - { - struct cache_level_desc *desc = &cache_desc.dcache[level]; - unsigned addr; - unsigned max_addr = 1 << desc->width; - unsigned way_sh = 32 - desc->log2_nways; - - /* D-cache clean by set-ways. */ - for (addr = (level << 1); addr < max_addr; addr += cache_desc.dminline) - { - unsigned int way; - - for (way = 0; way < desc->nways; way++) - stlink_write_debug32(sl, DCCSW, addr | (way << way_sh)); - } - } - - /* Invalidate all I-cache to oPU. */ - if (ccr & CCR_IC) - stlink_write_debug32(sl, ICIALLU, 0); + int level; + + if (ccr & CCR_DC) + for (level = cache_desc.louu - 1; level >= 0; level--) { + struct cache_level_desc *desc = &cache_desc.dcache[level]; + unsigned addr; + unsigned max_addr = 1 << desc->width; + unsigned way_sh = 32 - desc->log2_nways; + + /* D-cache clean by set-ways. */ + for (addr = (level << 1); addr < max_addr; addr += cache_desc.dminline) { + unsigned int way; + + for (way = 0; way < desc->nways; way++) + stlink_write_debug32(sl, DCCSW, addr | (way << way_sh)); + } + } + + /* Invalidate all I-cache to oPU. */ + if (ccr & CCR_IC) + stlink_write_debug32(sl, ICIALLU, 0); } static int cache_modified; @@ -1055,8 +1061,8 @@ static size_t unhexify(const char *in, char *out, size_t out_count) } int serve(stlink_t *sl, st_state_t *st) { - int sock = socket(AF_INET, SOCK_STREAM, 0); - if(sock < 0) { + SOCKET sock = socket(AF_INET, SOCK_STREAM, 0); + if(!IS_SOCK_VALID(sock)) { perror("socket"); return 1; } @@ -1072,28 +1078,27 @@ int serve(stlink_t *sl, st_state_t *st) { if(bind(sock, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) { perror("bind"); + close_socket(sock); return 1; } if(listen(sock, 5) < 0) { perror("listen"); + close_socket(sock); return 1; } ILOG("Listening at *:%d...\n", st->listen_port); - int client = accept(sock, NULL, NULL); + SOCKET client = accept(sock, NULL, NULL); //signal (SIGINT, SIG_DFL); - if(client < 0) { + if(!IS_SOCK_VALID(client)) { perror("accept"); + close_socket(sock); return 1; } -#if defined(__MINGW32__) || defined(_MSC_VER) - win32_close_socket(sock); -#else - close(sock); -#endif + close_socket(sock); stlink_force_debug(sl); if (st->reset) { @@ -1116,9 +1121,7 @@ int serve(stlink_t *sl, st_state_t *st) { int status = gdb_recv_packet(client, &packet); if(status < 0) { ELOG("cannot recv: %d\n", status); -#if defined(__MINGW32__) || defined(_MSC_VER) - win32_close_socket(client); -#endif + close_socket(client); return 1; } @@ -1343,6 +1346,8 @@ int serve(stlink_t *sl, st_state_t *st) { } else { reply = strdup("OK"); } + + free(decoded); } else if(!strcmp(cmdName, "FlashDone")) { if(flash_go(sl) < 0) { reply = strdup("E00"); @@ -1369,9 +1374,7 @@ int serve(stlink_t *sl, st_state_t *st) { status = gdb_check_for_interrupt(client); if(status < 0) { ELOG("cannot check for int: %d\n", status); -#if defined(__MINGW32__) || defined(_MSC_VER) - win32_close_socket(client); -#endif + close_socket(client); return 1; } @@ -1771,9 +1774,7 @@ int serve(stlink_t *sl, st_state_t *st) { ELOG("cannot send: %d\n", result); free(reply); free(packet); -#if defined(__MINGW32__) || defined(_MSC_VER) - win32_close_socket(client); -#endif + close_socket(client); return 1; } @@ -1783,9 +1784,7 @@ int serve(stlink_t *sl, st_state_t *st) { free(packet); } -#if defined(__MINGW32__) || defined(_MSC_VER) - win32_close_socket(client); -#endif + close_socket(client); return 0; } diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index b7792007f..9e92cf3bd 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -320,7 +320,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { int fd; uint32_t buffer_len; void *buffer; - ssize_t read_result; + ssize_t read_result; if (mem_read(sl, r1, args, sizeof (args)) != 0 ) { DLOG("Semihosting SYS_READ error: " diff --git a/src/mingw/mingw.c b/src/mingw/mingw.c index bfdd3dcff..4cb917ac4 100644 --- a/src/mingw/mingw.c +++ b/src/mingw/mingw.c @@ -15,7 +15,8 @@ int win32_poll(struct pollfd *fds, unsigned int nfds, int timo) { struct timeval timeout, *toptr; fd_set ifds, ofds, efds, *ip, *op; - unsigned int i, rc; + unsigned int i; + int rc; /* Set up the file-descriptor sets in ifds, ofds and efds. */ #ifdef _MSC_VER diff --git a/src/mingw/mingw.h b/src/mingw/mingw.h index 4fe07e611..e3a7267a9 100644 --- a/src/mingw/mingw.h +++ b/src/mingw/mingw.h @@ -60,7 +60,7 @@ SOCKET win32_socket(int, int, int); int win32_connect(SOCKET, struct sockaddr*, socklen_t); SOCKET win32_accept(SOCKET, struct sockaddr*, socklen_t *); int win32_shutdown(SOCKET, int); -int win32_close_socket(SOCKET fd); +int win32_close_socket(SOCKET fd); #define strtok_r(x, y, z) win32_strtok_r(x, y, z) #define strsep(x,y) win32_strsep(x,y) diff --git a/src/mmap.c b/src/mmap.c index cc5346f18..90ae5fc17 100644 --- a/src/mmap.c +++ b/src/mmap.c @@ -15,7 +15,11 @@ void *mmap (void *addr, size_t len, int prot, int flags, int fd, long long offs buf = malloc(len); if ( NULL == buf ) return MAP_FAILED; - if (lseek(fd,offset,SEEK_SET) != offset) return MAP_FAILED; + if (lseek(fd,offset,SEEK_SET) != offset) { + free(buf); + return MAP_FAILED; + } + count = read(fd, buf, len); diff --git a/src/sg.c b/src/sg.c index 285e2bd90..e7ff5d14e 100644 --- a/src/sg.c +++ b/src/sg.c @@ -945,7 +945,6 @@ static stlink_backend_t _stlink_sg_backend = { static stlink_t* stlink_open(const int verbose) { stlink_t *sl = malloc(sizeof (stlink_t)); - memset(sl, 0, sizeof(stlink_t)); struct stlink_libsg *slsg = malloc(sizeof (struct stlink_libsg)); if (sl == NULL || slsg == NULL) { WLOG("Couldn't malloc stlink and stlink_sg structures out of memory!\n"); @@ -955,6 +954,7 @@ static stlink_t* stlink_open(const int verbose) { free(slsg); return NULL; } + memset(sl, 0, sizeof(stlink_t)); if (libusb_init(&(slsg->libusb_ctx))) { WLOG("failed to init libusb context, wrong version of libraries?\n"); diff --git a/src/usb.c b/src/usb.c index c4d39ac5f..34586640e 100644 --- a/src/usb.c +++ b/src/usb.c @@ -625,7 +625,7 @@ int _stlink_usb_read_unsupported_reg(stlink_t *sl, int r_idx, struct stlink_reg if (ret == -1) return ret; - _stlink_usb_read_mem32(sl, STLINK_REG_DCRDR, 4); + ret = _stlink_usb_read_mem32(sl, STLINK_REG_DCRDR, 4); if (ret == -1) return ret;