From 38c3287f6831415efa4d50f7e819f445f8912c9e Mon Sep 17 00:00:00 2001 From: weetmuts Date: Fri, 4 Jan 2019 22:27:51 +0100 Subject: [PATCH] Fixed memory leak in shell. --- CHANGES | 8 ++++++++ Makefile | 12 ++++++------ check.sh | 6 ++++++ serial.cc | 7 +++++-- serial.h | 1 + shell.cc | 6 +++--- test.sh | 14 ++++++++++++++ wmbus.cc | 5 +---- wmbus_im871a.cc | 4 ++-- wmbus_simulator.cc | 4 ++-- wmbus_utils.cc | 3 +-- 11 files changed, 49 insertions(+), 21 deletions(-) create mode 100755 check.sh diff --git a/CHANGES b/CHANGES index c64b57b..ae2c69e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,12 @@ +Version 0.8.1: 2019-01-04 + +Fixed memory leak in shell invocation. +Improved dvparser to properly handle the supercom587 telegrams. +(It still does not extract all the data, but the data is properly parsed and chunked.) +Added address sanitizer to debug build. +Added static analysis check.sh. + Version 0.8: 2018-11-29 Multical21 now reports flow temperature and external temperature. diff --git a/Makefile b/Makefile index 5ad2609..385a31e 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,7 @@ endif $(shell mkdir -p $(BUILD)) -CXXFLAGS := $(DEBUG_FLAGS) -fPIC -fmessage-length=0 -std=c++11 -Wall -Wno-maybe-uninitialized -Wno-unused-function "-DWMBUSMETERS_VERSION=\"0.8\"" +CXXFLAGS := $(DEBUG_FLAGS) -fPIC -fmessage-length=0 -std=c++11 -Wall -Wno-maybe-uninitialized -Wno-unused-function "-DWMBUSMETERS_VERSION=\"0.8.1\"" $(BUILD)/%.o: %.cc $(wildcard %.h) $(CXX) $(CXXFLAGS) $< -c -o $@ @@ -72,20 +72,20 @@ METERS_OBJS:=\ all: $(BUILD)/wmbusmeters $(BUILD)/testinternals @$(STRIP_BINARY) -dist: wmbusmeters_0.8_$(DEBARCH).deb +dist: wmbusmeters_0.8.1_$(DEBARCH).deb -wmbusmeters_0.8_$(DEBARCH).deb: +wmbusmeters_0.8.1_$(DEBARCH).deb: @mkdir -p $(BUILD)/debian/wmbusmeters/DEBIAN @mkdir -p $(BUILD)/debian/wmbusmeters/usr/local/bin @cp $(BUILD)/wmbusmeters $(BUILD)/debian/wmbusmeters/usr/local/bin @rm -f $(BUILD)/debian/wmbusmeters/DEBIAN/control @echo "Package: wmbusmeters" >> $(BUILD)/debian/wmbusmeters/DEBIAN/control - @echo "Version: 0.8" >> $(BUILD)/debian/wmbusmeters/DEBIAN/control + @echo "Version: 0.8.1" >> $(BUILD)/debian/wmbusmeters/DEBIAN/control @echo "Maintainer: Fredrik Öhrström" >> $(BUILD)/debian/wmbusmeters/DEBIAN/control @echo "Architecture: $(DEBARCH)" >> $(BUILD)/debian/wmbusmeters/DEBIAN/control @echo "Description: A tool to read wireless mbus telegrams from utility meters." >> $(BUILD)/debian/wmbusmeters/DEBIAN/control @(cd $(BUILD)/debian; dpkg-deb --build wmbusmeters .) - @mv $(BUILD)/debian/wmbusmeters_0.8_$(DEBARCH).deb . + @mv $(BUILD)/debian/wmbusmeters_0.8.1_$(DEBARCH).deb . @echo Built package $@ $(BUILD)/wmbusmeters: $(METERS_OBJS) $(BUILD)/main.o @@ -96,7 +96,7 @@ $(BUILD)/testinternals: $(METERS_OBJS) $(BUILD)/testinternals.o $(CXX) -o $(BUILD)/testinternals $(METERS_OBJS) $(BUILD)/testinternals.o $(DEBUG_LDFLAGS) -lpthread clean: - rm -f build/* build_arm/* build_debug/* build_arm_debug/* *~ + rm -rf build/* build_arm/* build_debug/* build_arm_debug/* *~ test: ./build/testinternals diff --git a/check.sh b/check.sh new file mode 100755 index 0000000..32f2da4 --- /dev/null +++ b/check.sh @@ -0,0 +1,6 @@ +#!/bin/bash +# Build with static analysis tool Coverity +make clean +rm -rf cov-int +cov-build --dir cov-int make +tar czvf wmbusmeters.tgz cov-int diff --git a/serial.cc b/serial.cc index c05dc16..9c0b350 100644 --- a/serial.cc +++ b/serial.cc @@ -80,8 +80,8 @@ struct SerialDeviceTTY : public SerialDeviceImp { private: string device_; - int baud_rate_; - int fd_; + int baud_rate_ {}; + int fd_ {}; pthread_mutex_t write_lock_ = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_t read_lock_ = PTHREAD_MUTEX_INITIALIZER; SerialCommunicationManagerImp *manager_; @@ -208,6 +208,9 @@ unique_ptr SerialCommunicationManagerImp::createSerialDeviceTTY(st void SerialCommunicationManagerImp::listenTo(SerialDevice *sd, function cb) { SerialDeviceImp *si = dynamic_cast(sd); + if (!si) { + error("Internal error: Invalid serial device passed to listenTo.\n"); + } si->on_data_ = cb; } diff --git a/serial.h b/serial.h index 494a77a..fa3ed31 100644 --- a/serial.h +++ b/serial.h @@ -36,6 +36,7 @@ struct SerialDevice { virtual int receive(std::vector *data) = 0; virtual int fd() = 0; virtual SerialCommunicationManager *manager() = 0; + virtual ~SerialDevice() = default; }; struct SerialCommunicationManager { diff --git a/shell.cc b/shell.cc index 2b238d2..d0dc9fe 100644 --- a/shell.cc +++ b/shell.cc @@ -27,7 +27,7 @@ void invokeShell(string program, vector args, vector envs) { - const char **argv = new const char*[args.size()+2]; + vector argv(args.size()+2); argv[0] = program.c_str(); int i = 1; debug("exec \"%s\"\n", program.c_str()); @@ -38,7 +38,7 @@ void invokeShell(string program, vector args, vector envs) } argv[i] = NULL; - const char **env = new const char*[envs.size()+1]; + vector env(envs.size()+1); env[0] = program.c_str(); i = 0; for (auto &e : envs) { @@ -53,7 +53,7 @@ void invokeShell(string program, vector args, vector envs) if (pid == 0) { // I am the child! close(0); // Close stdin - execvpe(program.c_str(), (char*const*)argv, (char*const*)env); + execvpe(program.c_str(), (char*const*)&argv[0], (char*const*)&env[0]); perror("Execvp failed:"); error("Invoking shell %s failed!\n", program.c_str()); } else { diff --git a/test.sh b/test.sh index 25eeebc..78a9d08 100755 --- a/test.sh +++ b/test.sh @@ -43,3 +43,17 @@ then else Failure. fi + +$PROG --shell='echo "$METER_JSON"' simulation_shell.txt MWW supercom587 12345678 "" > test_output.txt +if [ "$?" == "0" ] +then + cat test_output.txt | sed 's/"timestamp":"....-..-..T..:..:..Z"/"timestamp":"1111-11-11T11:11:11Z"/' > test_responses.txt + echo '{"media":"warm water","meter":"supercom587","name":"MWW","id":"12345678","total_m3":5.548000,"timestamp":"1111-11-11T11:11:11Z"}' > test_expected.txt + diff test_expected.txt test_responses.txt + if [ "$?" == "0" ] + then + echo SHELL OK + fi +else + Failure. +fi diff --git a/wmbus.cc b/wmbus.cc index ea2b38c..0d954d9 100644 --- a/wmbus.cc +++ b/wmbus.cc @@ -655,7 +655,6 @@ string vifType(int vif) case 0x78: return "Fabrication no"; case 0x79: return "Enhanced identification"; - case 0x80: return "Address"; case 0x7C: return "VIF in following string (length in first byte)"; case 0x7E: return "Any VIF"; @@ -988,7 +987,6 @@ string vifKey(int vif) case 0x78: return "fabrication_no"; // Fabrication no case 0x79: return "enhanced_identification"; // Enhanced identification - case 0x80: return "address"; // Address case 0x7C: // VIF in following string (length in first byte) case 0x7E: // Any VIF @@ -1146,7 +1144,6 @@ string vifUnit(int vif) case 0x78: return ""; // Fabrication no case 0x79: return ""; // Enhanced identification - case 0x80: return ""; // Address case 0x7C: // VIF in following string (length in first byte) case 0x7E: // Any VIF @@ -1530,7 +1527,7 @@ string vif_FB_ExtensionType(uchar dif, uchar vif, uchar vife) return s; } - if ((vife & 0x7e) == 0x29 || + if ((vife & 0x7f) == 0x29 || (vife & 0x7c) == 0x2c) { return "Reserved"; } diff --git a/wmbus_im871a.cc b/wmbus_im871a.cc index c581fd6..39343c4 100644 --- a/wmbus_im871a.cc +++ b/wmbus_im871a.cc @@ -48,8 +48,8 @@ private: vector read_buffer_; pthread_mutex_t command_lock_ = PTHREAD_MUTEX_INITIALIZER; sem_t command_wait_; - int sent_command_; - int received_command_; + int sent_command_ {}; + int received_command_ {}; vector received_payload_; vector> telegram_listeners_; diff --git a/wmbus_simulator.cc b/wmbus_simulator.cc index 9856d7d..9262bdb 100644 --- a/wmbus_simulator.cc +++ b/wmbus_simulator.cc @@ -45,8 +45,8 @@ private: vector> telegram_listeners_; string file_; - SerialCommunicationManager *manager_; - LinkMode link_mode_; + SerialCommunicationManager *manager_ {}; + LinkMode link_mode_ {}; vector lines_; }; diff --git a/wmbus_utils.cc b/wmbus_utils.cc index 424b095..728ae34 100644 --- a/wmbus_utils.cc +++ b/wmbus_utils.cc @@ -120,14 +120,13 @@ void decryptMode5_AES_CBC(Telegram *t, vector &aeskey) uchar decrypted_data[content.size()]; AES_CBC_decrypt_buffer(decrypted_data, &content[0], content.size(), &aeskey[0], iv); - vector decrypted(decrypted_data, decrypted_data+content.size()); if (decrypted_data[0] != 0x2F || decrypted_data[1] != 0x2F) { verbose("(Mode5) decrypt failed!\n"); } t->content.clear(); - t->content.insert(t->content.end(), decrypted.begin(), decrypted.end()); + t->content.insert(t->content.end(), decrypted_data, decrypted_data+content.size()); debugPayload("(Mode5) decrypted", t->content); }