Fix memory leak and add EINTR error handling.

One memory leak was missed in the Installer service.
Introduce retries on EINTR error while trying to open a file.
Also add close() error handling.

This is a cherry pick from security-server repository.

Change-Id: I43b48e12d6e609b24ffda02c7aed199f3b9f02bb
Signed-off-by: Marcin Lis <m.lis@samsung.com>

diff --git a/src/server/service/installer.cpp b/src/server/service/installer.cpp
index 20829fd..15faa2d 100644 (file)
--- a/src/server/service/installer.cpp
+++ b/src/server/service/installer.cpp
@@ -99,24 +99,25 @@ FileDecision labelLinksToExecs(const FTSENT *ftsent)
     LogSecureDebug("Entering function: " << __func__);
 
     struct stat buf;
-    char *target;
 
     // check if it's a link
     if ( !S_ISLNK(ftsent->fts_statp->st_mode))
         return FileDecision::SKIP;
 
-    target = realpath(ftsent->fts_path, NULL);
-    if (!target) {
+    std::unique_ptr<char, std::function<void(void*)>> target(realpath(ftsent->fts_path, NULL), free);
+
+    if (!target.get()) {
         LogSecureError("Getting link target for " << ftsent->fts_path << " failed (Error = " << strerror(errno) << ")");
         return FileDecision::ERROR;
     }
-    if (-1 == stat(target, &buf)) {
-        LogSecureError("stat failed for " << target << " (Error = " << strerror(errno) << ")");
+
+    if (-1 == stat(target.get(), &buf)) {
+        LogSecureError("stat failed for " << target.get() << " (Error = " << strerror(errno) << ")");
         return FileDecision::ERROR;
     }
     // skip if link target is not a regular executable file
     if (buf.st_mode != (buf.st_mode | S_IXUSR | S_IFREG)) {
-        LogSecureDebug(target << "is not a regular executable file. Skipping.");
+        LogSecureDebug(target.get() << "is not a regular executable file. Skipping.");
         return FileDecision::SKIP;
     }
 

diff --git a/src/server/service/smack-rules.cpp b/src/server/service/smack-rules.cpp
index 4319ca9..687795e 100644 (file)
--- a/src/server/service/smack-rules.cpp
+++ b/src/server/service/smack-rules.cpp
@@ -29,6 +29,7 @@
 #include <sys/smack.h>
 #include <fcntl.h>
 #include <fstream>
+#include <cstring>
 
 #include <dpl/log/log.h>
 
@@ -74,7 +75,7 @@ bool SmackRules::loadFromFile(const std::string &path)
     int fd;
     bool ret = true;
 
-    fd = open(path.c_str(), O_RDONLY);
+    fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY));
     if (fd == -1) {
         LogError("Failed to open file: %s" << path);
         return false;
@@ -85,7 +86,11 @@ bool SmackRules::loadFromFile(const std::string &path)
         ret = false;
     }
 
-    close(fd);
+    if (close(fd) == -1) {
+        // don't change the return code, the descriptor should be closed despite the error.
+        LogWarning("Error while closing the file: " << path << ", error: " << strerror(errno));
+    }
+
     return ret;
 }
 
@@ -94,7 +99,7 @@ bool SmackRules::saveToFile(const std::string &path) const
     int fd;
     bool ret = true;
 
-    fd = open(path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0644);
+    fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0644));
     if (fd == -1) {
         LogError("Failed to create file: %s" << path);
         return false;
@@ -106,7 +111,18 @@ bool SmackRules::saveToFile(const std::string &path) const
         ret = false;
     }
 
-    close(fd);
+    if (close(fd) == -1) {
+        if (errno == EIO) {
+            LogError("I/O Error occured while closing the file: " << path << ", error: " << strerror(errno));
+            unlink(path.c_str());
+            return false;
+        } else {
+            // non critical error
+            // don't change the return code, the descriptor should be closed despite the error.
+            LogWarning("Error while closing the file: " << path << ", error: " << strerror(errno));
+        }
+    }
+
     return ret;
 }