Resolves: #1938810 - fix resource leaks reported by Coverity
This commit is contained in:
		
							parent
							
								
									e5f94db067
								
							
						
					
					
						commit
						d5a2a1ebe2
					
				
							
								
								
									
										615
									
								
								0001-logrotate-3.18.0-fix-resource-leaks.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										615
									
								
								0001-logrotate-3.18.0-fix-resource-leaks.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,615 @@ | |||||||
|  | From 471cf0a6a90e5d45f116f404e1276ea730dbece6 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Fri, 26 Mar 2021 17:18:09 +0100 | ||||||
|  | Subject: [PATCH 1/9] Do not report OOM incorrectly | ||||||
|  | 
 | ||||||
|  | In case there is no file in the set to rotate `calloc(0, ...)` is called | ||||||
|  | , which might return NULL. | ||||||
|  | 
 | ||||||
|  | Order the check for a zero number of files first, to void calling calloc | ||||||
|  | with a size of zero. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 7b65b267d73970eb59061be907c8c35b4396ada9 | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  logrotate.c | 12 ++++++------ | ||||||
|  |  1 file changed, 6 insertions(+), 6 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index 507c85a..a8c8480 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -2212,11 +2212,6 @@ static int rotateLogSet(const struct logInfo *log, int force)
 | ||||||
|  |      struct logState **state; | ||||||
|  |      struct logNames **rotNames; | ||||||
|  |   | ||||||
|  | -    logHasErrors = calloc(log->numFiles, sizeof(int));
 | ||||||
|  | -    if (!logHasErrors) {
 | ||||||
|  | -        message_OOM();
 | ||||||
|  | -        return 1;
 | ||||||
|  | -    }
 | ||||||
|  |      message(MESS_DEBUG, "\nrotating pattern: %s ", log->pattern); | ||||||
|  |      if (force) { | ||||||
|  |          message(MESS_DEBUG, "forced from command line "); | ||||||
|  | @@ -2277,10 +2272,15 @@ static int rotateLogSet(const struct logInfo *log, int force)
 | ||||||
|  |   | ||||||
|  |      if (log->numFiles == 0) { | ||||||
|  |          message(MESS_DEBUG, "No logs found. Rotation not needed.\n"); | ||||||
|  | -        free(logHasErrors);
 | ||||||
|  |          return 0; | ||||||
|  |      } | ||||||
|  |   | ||||||
|  | +    logHasErrors = calloc(log->numFiles, sizeof(int));
 | ||||||
|  | +    if (!logHasErrors) {
 | ||||||
|  | +        message_OOM();
 | ||||||
|  | +        return 1;
 | ||||||
|  | +    }
 | ||||||
|  | +
 | ||||||
|  |      if (log->flags & LOG_FLAG_SU) { | ||||||
|  |          if (switch_user(log->suUid, log->suGid) != 0) { | ||||||
|  |              free(logHasErrors); | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From 96203f4cdc64e2df3d203231bd1247424a20875e Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Mon, 19 Apr 2021 15:35:37 +0200 | ||||||
|  | Subject: [PATCH 2/9] Unify asprintf usage | ||||||
|  | 
 | ||||||
|  | Unify the error checking of asprintf(3). | ||||||
|  | 
 | ||||||
|  | Also reset the target string pointer to NULL on error, if it is non- | ||||||
|  | local, since the content is undefined according to the specification. | ||||||
|  | 
 | ||||||
|  | Also fix potential NULL-pointer usage in sprintf(3): | ||||||
|  | 
 | ||||||
|  | logrotate.c:1595: | ||||||
|  |     rotNames->dirName = malloc(strlen(ld) + strlen(log->oldDir) + 2); | ||||||
|  |     sprintf(rotNames->dirName, "%s/%s", ld, log->oldDir); | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 5afcdeecc5a3bfe07671a3c05c7a301da9206ccd | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  config.c    | 28 +++++++++++++--------------- | ||||||
|  |  logrotate.c |  9 ++++++--- | ||||||
|  |  2 files changed, 19 insertions(+), 18 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/config.c b/config.c
 | ||||||
|  | index df2d90b..19dcfce 100644
 | ||||||
|  | --- a/config.c
 | ||||||
|  | +++ b/config.c
 | ||||||
|  | @@ -815,21 +815,19 @@ int readAllConfigPaths(const char **paths)
 | ||||||
|  |   | ||||||
|  |   | ||||||
|  |      for (i = 0; i < defTabooCount; i++) { | ||||||
|  | -        int bytes;
 | ||||||
|  |          char *pattern = NULL; | ||||||
|  |   | ||||||
|  |          /* generate a pattern by concatenating star (wildcard) to the | ||||||
|  |           * suffix literal | ||||||
|  |           */ | ||||||
|  | -        bytes = asprintf(&pattern, "*%s", defTabooExts[i]);
 | ||||||
|  | -        if (bytes != -1) {
 | ||||||
|  | -            tabooPatterns[i] = pattern;
 | ||||||
|  | -            tabooCount++;
 | ||||||
|  | -        } else {
 | ||||||
|  | +        if (asprintf(&pattern, "*%s", defTabooExts[i]) < 0) {
 | ||||||
|  |              free_2d_array(tabooPatterns, tabooCount); | ||||||
|  |              message_OOM(); | ||||||
|  |              return 1; | ||||||
|  |          } | ||||||
|  | +
 | ||||||
|  | +        tabooPatterns[i] = pattern;
 | ||||||
|  | +        tabooCount++;
 | ||||||
|  |      } | ||||||
|  |   | ||||||
|  |      for (file = paths; *file; file++) { | ||||||
|  | @@ -1421,7 +1419,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                          } | ||||||
|  |   | ||||||
|  |                          while (*endtag) { | ||||||
|  | -                            int bytes;
 | ||||||
|  |                              char *pattern = NULL; | ||||||
|  |   | ||||||
|  |                              chptr = endtag; | ||||||
|  | @@ -1437,10 +1434,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                                      RAISE_ERROR(); | ||||||
|  |                                  } | ||||||
|  |                                  tabooPatterns = tmp; | ||||||
|  | -                                bytes = asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag);
 | ||||||
|  | +                                if (asprintf(&pattern, "*%.*s", (int)(chptr - endtag), endtag) < 0) {
 | ||||||
|  | +                                    message_OOM();
 | ||||||
|  | +                                    RAISE_ERROR();
 | ||||||
|  | +                                }
 | ||||||
|  |   | ||||||
|  | -                                /* should test for malloc() failure */
 | ||||||
|  | -                                assert(bytes != -1);
 | ||||||
|  |                                  tabooPatterns[tabooCount] = pattern; | ||||||
|  |                                  tabooCount++; | ||||||
|  |                              } | ||||||
|  | @@ -1481,7 +1479,6 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                          } | ||||||
|  |   | ||||||
|  |                          while (*endtag) { | ||||||
|  | -                            int bytes;
 | ||||||
|  |                              char *pattern = NULL; | ||||||
|  |                              char **tmp; | ||||||
|  |   | ||||||
|  | @@ -1496,10 +1493,11 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                                  RAISE_ERROR(); | ||||||
|  |                              } | ||||||
|  |                              tabooPatterns = tmp; | ||||||
|  | -                            bytes = asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag);
 | ||||||
|  | +                            if (asprintf(&pattern, "%.*s", (int)(chptr - endtag), endtag) < 0) {
 | ||||||
|  | +                                message_OOM();
 | ||||||
|  | +                                RAISE_ERROR();
 | ||||||
|  | +                            }
 | ||||||
|  |   | ||||||
|  | -                            /* should test for malloc() failure */
 | ||||||
|  | -                            assert(bytes != -1);
 | ||||||
|  |                              tabooPatterns[tabooCount] = pattern; | ||||||
|  |                              tabooCount++; | ||||||
|  |   | ||||||
|  | @@ -1540,7 +1538,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                                  env_home = pwd->pw_dir; | ||||||
|  |                              } | ||||||
|  |   | ||||||
|  | -                            if (asprintf(&new_key, "%s/%s", env_home, key + 2) == -1) {
 | ||||||
|  | +                            if (asprintf(&new_key, "%s/%s", env_home, key + 2) < 0) {
 | ||||||
|  |                                  message_OOM(); | ||||||
|  |                                  RAISE_ERROR(); | ||||||
|  |                              } | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index a8c8480..e294352 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -1576,9 +1576,9 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |          ld = dirname(logpath); | ||||||
|  |          if (log->oldDir) { | ||||||
|  |              if (log->oldDir[0] != '/') { | ||||||
|  | -                rotNames->dirName =
 | ||||||
|  | -                    malloc(strlen(ld) + strlen(log->oldDir) + 2);
 | ||||||
|  | -                sprintf(rotNames->dirName, "%s/%s", ld, log->oldDir);
 | ||||||
|  | +                if (asprintf(&rotNames->dirName, "%s/%s", ld, log->oldDir) < 0) {
 | ||||||
|  | +                    rotNames->dirName = NULL;
 | ||||||
|  | +                }
 | ||||||
|  |              } else | ||||||
|  |                  rotNames->dirName = strdup(log->oldDir); | ||||||
|  |          } else | ||||||
|  | @@ -1983,6 +1983,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |          if (asprintf(&(rotNames->finalName), "%s/%s%s%s", rotNames->dirName, | ||||||
|  |                       rotNames->baseName, dext_str, fileext) < 0) { | ||||||
|  |              message_OOM(); | ||||||
|  | +            rotNames->finalName = NULL;
 | ||||||
|  |              return 1; | ||||||
|  |          } | ||||||
|  |          if (asprintf(&destFile, "%s%s", rotNames->finalName, compext) < 0) { | ||||||
|  | @@ -2001,6 +2002,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |          if (asprintf(&(rotNames->finalName), "%s/%s.%d%s", rotNames->dirName, | ||||||
|  |                       rotNames->baseName, logStart, fileext) < 0) { | ||||||
|  |              message_OOM(); | ||||||
|  | +            rotNames->finalName = NULL;
 | ||||||
|  |          } | ||||||
|  |      } | ||||||
|  |   | ||||||
|  | @@ -2084,6 +2086,7 @@ static int rotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                  free(rotNames->disposeName); | ||||||
|  |                  if (asprintf(&rotNames->disposeName, "%s%s", rotNames->finalName, ext) < 0) { | ||||||
|  |                      message_OOM(); | ||||||
|  | +                    rotNames->disposeName = NULL;
 | ||||||
|  |                      return 1; | ||||||
|  |                  } | ||||||
|  |   | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From 3cf921e0d58993b064cd6d52b44835008345f498 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Mon, 19 Apr 2021 15:40:19 +0200 | ||||||
|  | Subject: [PATCH 3/9] Update custom asprintf implementation | ||||||
|  | 
 | ||||||
|  | Check for vsnprintf(3) failures. | ||||||
|  | 
 | ||||||
|  | Silence conversion warnings. | ||||||
|  | 
 | ||||||
|  | Do not call exit(2) on allocation failure, but return -1 like the | ||||||
|  | specification says. All callers check the return value, since they | ||||||
|  | need to handle standard asprintf(3) implementations. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: f917b31dbb47992bf5c5342c7312ddb2e64efc40 | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  config.c | 20 ++++++++------------ | ||||||
|  |  1 file changed, 8 insertions(+), 12 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/config.c b/config.c
 | ||||||
|  | index 19dcfce..0d79980 100644
 | ||||||
|  | --- a/config.c
 | ||||||
|  | +++ b/config.c
 | ||||||
|  | @@ -61,24 +61,20 @@ int asprintf(char **string_ptr, const char *format, ...)
 | ||||||
|  |   | ||||||
|  |      va_start(arg, format); | ||||||
|  |      size = vsnprintf(NULL, 0, format, arg); | ||||||
|  | -    size++;
 | ||||||
|  |      va_end(arg); | ||||||
|  | -    va_start(arg, format);
 | ||||||
|  | -    str = malloc(size);
 | ||||||
|  | +    if (size < 0) {
 | ||||||
|  | +        return -1;
 | ||||||
|  | +    }
 | ||||||
|  | +    str = malloc((size_t)size + 1);
 | ||||||
|  |      if (str == NULL) { | ||||||
|  | -        va_end(arg);
 | ||||||
|  | -        /*
 | ||||||
|  | -         * Strictly speaking, GNU asprintf doesn't do this,
 | ||||||
|  | -         * but the caller isn't checking the return value.
 | ||||||
|  | -         */
 | ||||||
|  | -        message_OOM();
 | ||||||
|  | -        exit(1);
 | ||||||
|  | +        return -1;
 | ||||||
|  |      } | ||||||
|  | -    rv = vsnprintf(str, size, format, arg);
 | ||||||
|  | +    va_start(arg, format);
 | ||||||
|  | +    rv = vsnprintf(str, (size_t)size + 1, format, arg);
 | ||||||
|  |      va_end(arg); | ||||||
|  |   | ||||||
|  |      *string_ptr = str; | ||||||
|  | -    return (rv);
 | ||||||
|  | +    return rv;
 | ||||||
|  |  } | ||||||
|  |   | ||||||
|  |  #endif | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From ace9818a606a0c96bb6e4da479ed151650b8fa3a Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Mon, 19 Apr 2021 15:45:55 +0200 | ||||||
|  | Subject: [PATCH 4/9] Use asprintf instead of split malloc + sprintf | ||||||
|  | 
 | ||||||
|  | Use asprintf(3) instead of split usage of malloc(3) and sprintf(3) to | ||||||
|  | reduce the chance of potential size inconsistencies. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 001352baa924f021748513b6d09d37eca754d5cc | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  config.c    |  5 ++--- | ||||||
|  |  logrotate.c | 25 ++++++++++++------------- | ||||||
|  |  2 files changed, 14 insertions(+), 16 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/config.c b/config.c
 | ||||||
|  | index 0d79980..2905ff7 100644
 | ||||||
|  | --- a/config.c
 | ||||||
|  | +++ b/config.c
 | ||||||
|  | @@ -1886,13 +1886,12 @@ duperror:
 | ||||||
|  |                                      continue; | ||||||
|  |                                  } | ||||||
|  |                              } | ||||||
|  | -                            ld = malloc(strlen(dirName) + strlen(newlog->oldDir) + 2);
 | ||||||
|  | -                            if (ld == NULL) {
 | ||||||
|  | +                            if (asprintf(&ld, "%s/%s", dirName, newlog->oldDir) < 0) {
 | ||||||
|  |                                  message_OOM(); | ||||||
|  |                                  free(dirpath); | ||||||
|  |                                  goto error; | ||||||
|  |                              } | ||||||
|  | -                            sprintf(ld, "%s/%s", dirName, newlog->oldDir);
 | ||||||
|  | +
 | ||||||
|  |                              free(dirpath); | ||||||
|  |   | ||||||
|  |                              if (newlog->oldDir[0] != '/') { | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index e294352..a72329e 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -1810,15 +1810,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |          } | ||||||
|  |      } | ||||||
|  |   | ||||||
|  | -    /* adding 2 due to / and \0 being added by snprintf */
 | ||||||
|  | -    rotNames->firstRotated =
 | ||||||
|  | -        malloc(strlen(rotNames->dirName) + strlen(rotNames->baseName) +
 | ||||||
|  | -                strlen(fileext) + strlen(compext) + DATEEXT_LEN + 2 );
 | ||||||
|  | -    if (rotNames->firstRotated == NULL) {
 | ||||||
|  | -        message_OOM();
 | ||||||
|  | -        return 1;
 | ||||||
|  | -    }
 | ||||||
|  | -
 | ||||||
|  |      if (log->flags & LOG_FLAG_DATEEXT) { | ||||||
|  |          /* glob for compressed files with our pattern | ||||||
|  |           * and compress ext */ | ||||||
|  | @@ -1882,9 +1873,13 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |              rotNames->disposeName = NULL; | ||||||
|  |          } | ||||||
|  |          /* firstRotated is most recently created/compressed rotated log */ | ||||||
|  | -        sprintf(rotNames->firstRotated, "%s/%s%s%s%s",
 | ||||||
|  | +        if (asprintf(&rotNames->firstRotated, "%s/%s%s%s%s",
 | ||||||
|  |                  rotNames->dirName, rotNames->baseName, dext_str, fileext, | ||||||
|  | -                (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext);
 | ||||||
|  | +                (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
 | ||||||
|  | +            message_OOM();
 | ||||||
|  | +            rotNames->firstRotated = NULL;
 | ||||||
|  | +            return 1;
 | ||||||
|  | +        }
 | ||||||
|  |          globfree(&globResult); | ||||||
|  |          free(glob_pattern); | ||||||
|  |      } else { | ||||||
|  | @@ -1915,9 +1910,13 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |              } | ||||||
|  |          } | ||||||
|  |   | ||||||
|  | -        sprintf(rotNames->firstRotated, "%s/%s.%d%s%s", rotNames->dirName,
 | ||||||
|  | +        if (asprintf(&rotNames->firstRotated, "%s/%s.%d%s%s", rotNames->dirName,
 | ||||||
|  |                  rotNames->baseName, logStart, fileext, | ||||||
|  | -                (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext);
 | ||||||
|  | +                (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) {
 | ||||||
|  | +            message_OOM();
 | ||||||
|  | +            rotNames->firstRotated = NULL;
 | ||||||
|  | +            return 1;
 | ||||||
|  | +        }
 | ||||||
|  |   | ||||||
|  |          for (i = rotateCount + logStart - 1; (i >= 0) && !hasErrors; i--) { | ||||||
|  |              free(newName); | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From e8a655ef1977add152d79c4dc8148fe7b1c9bca2 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Mon, 19 Apr 2021 17:52:48 +0200 | ||||||
|  | Subject: [PATCH 5/9] Mark read-only string variable const | ||||||
|  | 
 | ||||||
|  | Prevent it accidentally being passed to free(3) or similar. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 2231aba823ff6e5a18d996e81ef63df0871224dd | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  logrotate.c | 2 +- | ||||||
|  |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index a72329e..7d49261 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -1567,7 +1567,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |      state->lastRotated = now; | ||||||
|  |   | ||||||
|  |      { | ||||||
|  | -        char *ld;
 | ||||||
|  | +        const char *ld;
 | ||||||
|  |          char *logpath = strdup(log->files[logNum]); | ||||||
|  |          if (logpath == NULL) { | ||||||
|  |              message_OOM(); | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From c06f20f781c74b2256e8f1757433db7e043b4ddf Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Mon, 19 Apr 2021 17:59:21 +0200 | ||||||
|  | Subject: [PATCH 6/9] Limit scope of variable | ||||||
|  | 
 | ||||||
|  | Limit the scope of a variable, by splitting it into several distinct | ||||||
|  | block scope variables. | ||||||
|  | 
 | ||||||
|  | This makes some asprintf(3) calls obsolete, and improves readability by | ||||||
|  | splitting the purpose of the variable. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: b37fb75f569b3ddde30dd85184ea160f63abb7d5 | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  logrotate.c | 19 ++++++++----------- | ||||||
|  |  1 file changed, 8 insertions(+), 11 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index 7d49261..962ac55 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -1529,7 +1529,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                                struct logState *state, struct logNames *rotNames) | ||||||
|  |  { | ||||||
|  |      struct tm now; | ||||||
|  | -    char *oldName = NULL;
 | ||||||
|  |      const char *compext = ""; | ||||||
|  |      const char *fileext = ""; | ||||||
|  |      int hasErrors = 0; | ||||||
|  | @@ -1770,11 +1769,8 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                  sortGlobResult(&globResult, strlen(rotNames->dirName) + 1 + strlen(rotNames->baseName), dformat); | ||||||
|  |                  for (glob_count = 0; glob_count < globResult.gl_pathc && !hasErrors; glob_count++) { | ||||||
|  |                      struct stat sbprev; | ||||||
|  | +                    const char *oldName = globResult.gl_pathv[glob_count];
 | ||||||
|  |   | ||||||
|  | -                    if (asprintf(&oldName, "%s", (globResult.gl_pathv)[glob_count]) < 0) {
 | ||||||
|  | -                        message_OOM();
 | ||||||
|  | -                        return 1;
 | ||||||
|  | -                    }
 | ||||||
|  |                      if (stat(oldName, &sbprev)) { | ||||||
|  |                          if (errno == ENOENT) | ||||||
|  |                              message(MESS_DEBUG, "previous log %s does not exist\n", oldName); | ||||||
|  | @@ -1783,7 +1779,6 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                      } else { | ||||||
|  |                          hasErrors = compressLogFile(oldName, log, &sbprev); | ||||||
|  |                      } | ||||||
|  | -                    free(oldName);
 | ||||||
|  |                  } | ||||||
|  |              } else { | ||||||
|  |                  message(MESS_DEBUG, | ||||||
|  | @@ -1793,6 +1788,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |              free(glob_pattern); | ||||||
|  |          } else { | ||||||
|  |              struct stat sbprev; | ||||||
|  | +            char *oldName;
 | ||||||
|  |              if (asprintf(&oldName, "%s/%s.%d%s", rotNames->dirName, | ||||||
|  |                           rotNames->baseName, logStart, fileext) < 0) { | ||||||
|  |                  message_OOM(); | ||||||
|  | @@ -1853,16 +1849,14 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |              } | ||||||
|  |              if (mail_out != (size_t)-1) { | ||||||
|  |                  /* oldName is oldest Backup found (for unlink later) */ | ||||||
|  | -                if (asprintf(&oldName, "%s", (globResult.gl_pathv)[mail_out]) < 0) {
 | ||||||
|  | -                    message_OOM();
 | ||||||
|  | -                    return 1;
 | ||||||
|  | -                }
 | ||||||
|  | +                const char *oldName = globResult.gl_pathv[mail_out];
 | ||||||
|  |                  rotNames->disposeName = strdup(oldName); | ||||||
|  |                  if (rotNames->disposeName == NULL) { | ||||||
|  |                      message_OOM(); | ||||||
|  | +                    globfree(&globResult);
 | ||||||
|  | +                    free(glob_pattern);
 | ||||||
|  |                      return 1; | ||||||
|  |                  } | ||||||
|  | -                free(oldName);
 | ||||||
|  |              } else { | ||||||
|  |                  free(rotNames->disposeName); | ||||||
|  |                  rotNames->disposeName = NULL; | ||||||
|  | @@ -1878,6 +1872,8 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                  (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) { | ||||||
|  |              message_OOM(); | ||||||
|  |              rotNames->firstRotated = NULL; | ||||||
|  | +            globfree(&globResult);
 | ||||||
|  | +            free(glob_pattern);
 | ||||||
|  |              return 1; | ||||||
|  |          } | ||||||
|  |          globfree(&globResult); | ||||||
|  | @@ -1885,6 +1881,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |      } else { | ||||||
|  |          int i; | ||||||
|  |          char *newName = NULL; | ||||||
|  | +        char *oldName;
 | ||||||
|  |   | ||||||
|  |          if (rotateCount == -1) { | ||||||
|  |              rotateCount = findLastRotated(rotNames, fileext, compext); | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From 1a1eb69e6c4ae403edceb411cb0bbc80473e2527 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> | ||||||
|  | Date: Tue, 20 Apr 2021 17:41:16 +0200 | ||||||
|  | Subject: [PATCH 7/9] Free memory on noolddir configuration | ||||||
|  | 
 | ||||||
|  | Consider the following configuration: | ||||||
|  |     olddir /var/log/foo | ||||||
|  |     noolddir | ||||||
|  | 
 | ||||||
|  | Do not leak the memory of the initial olddir path. | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 59e8e321d3221a3beaf7b9c99b17d5cb7dbcfaf0 | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  config.c | 2 +- | ||||||
|  |  1 file changed, 1 insertion(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/config.c b/config.c
 | ||||||
|  | index 2905ff7..b7406f5 100644
 | ||||||
|  | --- a/config.c
 | ||||||
|  | +++ b/config.c
 | ||||||
|  | @@ -1134,7 +1134,7 @@ static int readConfigFile(const char *configFile, struct logInfo *defConfig)
 | ||||||
|  |                          if (newlog->dateformat == NULL) | ||||||
|  |                              continue; | ||||||
|  |                      } else if (!strcmp(key, "noolddir")) { | ||||||
|  | -                        newlog->oldDir = NULL;
 | ||||||
|  | +                        freeLogItem(oldDir);
 | ||||||
|  |                      } else if (!strcmp(key, "mailfirst")) { | ||||||
|  |                          newlog->flags |= LOG_FLAG_MAILFIRST; | ||||||
|  |                      } else if (!strcmp(key, "maillast")) { | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From 4aabfd0fe19832ba1df8919356d1d2d4b463937d Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | Date: Mon, 3 May 2021 15:14:09 +0200 | ||||||
|  | Subject: [PATCH 8/9] readConfigFile: release `globerr_msg` on all code paths | ||||||
|  | 
 | ||||||
|  | This eliminates the following reports by Coverity: | ||||||
|  | 
 | ||||||
|  | Error: RESOURCE_LEAK (CWE-772): | ||||||
|  | logrotate-3.18.0.18_7a4d/config.c:1798: alloc_arg: "asprintf" allocates memory that is stored into "globerr_msg". [Note: The source code implementation of the function has been overridden by a builtin model.] | ||||||
|  | logrotate-3.18.0.18_7a4d/config.c:2116: leaked_storage: Variable "globerr_msg" going out of scope leaks the storage it points to. | ||||||
|  | 
 | ||||||
|  | Error: RESOURCE_LEAK (CWE-772): | ||||||
|  | logrotate-3.18.0.18_7a4d/config.c:1798: alloc_arg: "asprintf" allocates memory that is stored into "globerr_msg". [Note: The source code implementation of the function has been overridden by a builtin model.] | ||||||
|  | logrotate-3.18.0.18_7a4d/config.c:2122: leaked_storage: Variable "globerr_msg" going out of scope leaks the storage it points to. | ||||||
|  | 
 | ||||||
|  | Closes: https://github.com/logrotate/logrotate/pull/387 | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 97f841be2bb52b9ac00cd564a6eb0a853d1017bd | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  config.c | 2 ++ | ||||||
|  |  1 file changed, 2 insertions(+) | ||||||
|  | 
 | ||||||
|  | diff --git a/config.c b/config.c
 | ||||||
|  | index b7406f5..91fd412 100644
 | ||||||
|  | --- a/config.c
 | ||||||
|  | +++ b/config.c
 | ||||||
|  | @@ -2086,12 +2086,14 @@ next_state: ;
 | ||||||
|  |   | ||||||
|  |      munmap(buf, length); | ||||||
|  |      close(fd); | ||||||
|  | +    free(globerr_msg);
 | ||||||
|  |      return logerror; | ||||||
|  |  error: | ||||||
|  |      /* free is a NULL-safe operation */ | ||||||
|  |      free(key); | ||||||
|  |      munmap(buf, length); | ||||||
|  |      close(fd); | ||||||
|  | +    free(globerr_msg);
 | ||||||
|  |      return 1; | ||||||
|  |  } | ||||||
|  |   | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | From b5610cd1b0bc2cf9ab887007a953fbf6340cb150 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | Date: Mon, 3 May 2021 15:17:59 +0200 | ||||||
|  | Subject: [PATCH 9/9] prerotateSingleLog: release `oldName` on all code paths | ||||||
|  | 
 | ||||||
|  | This eliminates the following reports by Coverity: | ||||||
|  | 
 | ||||||
|  | Error: RESOURCE_LEAK (CWE-772): | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1911: alloc_arg: "asprintf" allocates memory that is stored into "oldName". [Note: The source code implementation of the function has been overridden by a builtin model.] | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1919: noescape: Resource "oldName" is not freed or pointed-to in "strdup". | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1922: leaked_storage: Variable "oldName" going out of scope leaks the storage it points to. | ||||||
|  | 
 | ||||||
|  | Error: RESOURCE_LEAK (CWE-772): | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1911: alloc_arg: "asprintf" allocates memory that is stored into "oldName". [Note: The source code implementation of the function has been overridden by a builtin model.] | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1919: noescape: Resource "oldName" is not freed or pointed-to in "strdup". | ||||||
|  | logrotate-3.18.0.18_7a4d/logrotate.c:1931: leaked_storage: Variable "oldName" going out of scope leaks the storage it points to. | ||||||
|  | 
 | ||||||
|  | Closes: https://github.com/logrotate/logrotate/pull/387 | ||||||
|  | 
 | ||||||
|  | Upstream-commit: 85bc130b19344a3d9c8b472142df14ddcd0a166d | ||||||
|  | Signed-off-by: Kamil Dudka <kdudka@redhat.com> | ||||||
|  | ---
 | ||||||
|  |  logrotate.c | 2 ++ | ||||||
|  |  1 file changed, 2 insertions(+) | ||||||
|  | 
 | ||||||
|  | diff --git a/logrotate.c b/logrotate.c
 | ||||||
|  | index 962ac55..d3f2825 100644
 | ||||||
|  | --- a/logrotate.c
 | ||||||
|  | +++ b/logrotate.c
 | ||||||
|  | @@ -1903,6 +1903,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |              rotNames->disposeName = strdup(oldName); | ||||||
|  |              if (rotNames->disposeName == NULL) { | ||||||
|  |                  message_OOM(); | ||||||
|  | +                free(oldName);
 | ||||||
|  |                  return 1; | ||||||
|  |              } | ||||||
|  |          } | ||||||
|  | @@ -1911,6 +1912,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
 | ||||||
|  |                  rotNames->baseName, logStart, fileext, | ||||||
|  |                  (log->flags & LOG_FLAG_DELAYCOMPRESS) ? "" : compext) < 0) { | ||||||
|  |              message_OOM(); | ||||||
|  | +            free(oldName);
 | ||||||
|  |              rotNames->firstRotated = NULL; | ||||||
|  |              return 1; | ||||||
|  |          } | ||||||
|  | -- 
 | ||||||
|  | 2.30.2 | ||||||
|  | 
 | ||||||
| @ -1,12 +1,15 @@ | |||||||
| Summary: Rotates, compresses, removes and mails system log files | Summary: Rotates, compresses, removes and mails system log files | ||||||
| Name: logrotate | Name: logrotate | ||||||
| Version: 3.18.0 | Version: 3.18.0 | ||||||
| Release: 2%{?dist} | Release: 3%{?dist} | ||||||
| License: GPLv2+ | License: GPLv2+ | ||||||
| URL: https://github.com/logrotate/logrotate | URL: https://github.com/logrotate/logrotate | ||||||
| Source0: https://github.com/logrotate/logrotate/releases/download/%{version}/logrotate-%{version}.tar.xz | Source0: https://github.com/logrotate/logrotate/releases/download/%{version}/logrotate-%{version}.tar.xz | ||||||
| Source1: rwtab | Source1: rwtab | ||||||
| 
 | 
 | ||||||
|  | # fix resource leaks reported by Coverity | ||||||
|  | Patch:   0001-logrotate-3.18.0-fix-resource-leaks.patch | ||||||
|  | 
 | ||||||
| BuildRequires: acl | BuildRequires: acl | ||||||
| BuildRequires: automake | BuildRequires: automake | ||||||
| BuildRequires: gcc | BuildRequires: gcc | ||||||
| @ -107,6 +110,9 @@ fi | |||||||
| %config(noreplace) %{_sysconfdir}/rwtab.d/logrotate | %config(noreplace) %{_sysconfdir}/rwtab.d/logrotate | ||||||
| 
 | 
 | ||||||
| %changelog | %changelog | ||||||
|  | * Tue May 04 2021 Kamil Dudka <kdudka@redhat.com> - 3.18.0-3 | ||||||
|  | - fix resource leaks reported by Coverity | ||||||
|  | 
 | ||||||
| * Tue Jan 26 2021 Fedora Release Engineering <releng@fedoraproject.org> - 3.18.0-2 | * Tue Jan 26 2021 Fedora Release Engineering <releng@fedoraproject.org> - 3.18.0-2 | ||||||
| - Rebuilt for https://fedoraproject.org/wiki/Fedora_34_Mass_Rebuild | - Rebuilt for https://fedoraproject.org/wiki/Fedora_34_Mass_Rebuild | ||||||
| 
 | 
 | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user