Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 22 Feb 2019 15:31:51 +0000 (UTC)
From:      David Bright <dab@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r344470 - head/usr.sbin/newsyslog
Message-ID:  <201902221531.x1MFVpd4046080@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: dab
Date: Fri Feb 22 15:31:50 2019
New Revision: 344470
URL: https://svnweb.freebsd.org/changeset/base/344470

Log:
  Fix several Coverity-detected issues in newsyslog.
  
  - CID 1394815, CID 1305673: Dereference before null check - memory was
    allocated and the allocation checked for NULL with a call to errx()
    if it failed. Code below that was guaranteed that the pointer was
    non-NULL, but there was another check for NULL at the exit of the
    function (after the memory had already been referenced). Eliminate
    the useless NULL check.
  
  - CID 1007452: Resource leak - Storage intended to be allocated and
    returned to the caller was never freed. This was the result of a
    regression in the function signature introduced in r208648 (2010)
    (thanks for that find, @cem!). Fixed by altering the function
    signature and passing the allocated memory to the caller as
    intended. This also fixes PR158794.
  
  - CID 1008620: Logically dead code in newsyslog.c - This was a direct
    result of CID 1007452. Since the memory allocated as described there
    was not returned to the caller, a subsequent check for the memory
    having been allocated was dead code. Returning the memory
    re-animates the code that is the subject of this CID.
  
  - CID 1006131: Unused value - in parsing a configuration file, a
    pointer to the end of the last field was saved, but not used after
    that. Rewrite to use the pointer value. This could have been fixed
    by avoiding the assignment altogether, but this solutions more
    closely follows the pattern used in the preceding code.
  
  PR:		158794
  Reported by:	Coverity, Ken-ichi EZURA <k.ezura@gmail.com> (PR158794)
  Reviewed by:	cem, markj
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D19105

Modified:
  head/usr.sbin/newsyslog/newsyslog.c

Modified: head/usr.sbin/newsyslog/newsyslog.c
==============================================================================
--- head/usr.sbin/newsyslog/newsyslog.c	Fri Feb 22 15:15:36 2019	(r344469)
+++ head/usr.sbin/newsyslog/newsyslog.c	Fri Feb 22 15:31:50 2019	(r344470)
@@ -253,7 +253,7 @@ static const char *path_syslogpid = _PATH_SYSLOGPID;
 
 static struct cflist *get_worklist(char **files);
 static void parse_file(FILE *cf, struct cflist *work_p, struct cflist *glob_p,
-		    struct conf_entry *defconf_p, struct ilist *inclist);
+		    struct conf_entry **defconf, struct ilist *inclist);
 static void add_to_queue(const char *fname, struct ilist *inclist);
 static char *sob(char *p);
 static char *son(char *p);
@@ -841,7 +841,7 @@ get_worklist(char **files)
 
 		if (verbose)
 			printf("Processing %s\n", inc->file);
-		parse_file(f, filelist, globlist, defconf, &inclist);
+		parse_file(f, filelist, globlist, &defconf, &inclist);
 		(void) fclose(f);
 	}
 
@@ -858,7 +858,6 @@ get_worklist(char **files)
 		if (defconf != NULL)
 			free_entry(defconf);
 		return (filelist);
-		/* NOTREACHED */
 	}
 
 	/*
@@ -915,7 +914,7 @@ get_worklist(char **files)
 		 * for a "glob" entry which does match.
 		 */
 		gmatch = 0;
-		if (verbose > 2 && globlist != NULL)
+		if (verbose > 2)
 			printf("\t+ Checking globs for %s\n", *given);
 		STAILQ_FOREACH(ent, globlist, cf_nextp) {
 			fnres = fnmatch(ent->log, *given, FNM_PATHNAME);
@@ -1046,7 +1045,7 @@ expand_globs(struct cflist *work_p, struct cflist *glo
  */
 static void
 parse_file(FILE *cf, struct cflist *work_p, struct cflist *glob_p,
-    struct conf_entry *defconf_p, struct ilist *inclist)
+    struct conf_entry **defconf_p, struct ilist *inclist)
 {
 	char line[BUFSIZ], *parse, *q;
 	char *cp, *errline, *group;
@@ -1137,12 +1136,12 @@ parse_file(FILE *cf, struct cflist *work_p, struct cfl
 		working = init_entry(q, NULL);
 		if (strcasecmp(DEFAULT_MARKER, q) == 0) {
 			special = 1;
-			if (defconf_p != NULL) {
+			if (*defconf_p != NULL) {
 				warnx("Ignoring duplicate entry for %s!", q);
 				free_entry(working);
 				continue;
 			}
-			defconf_p = working;
+			*defconf_p = working;
 		}
 
 		q = parse = missing_field(sob(parse + 1), errline);
@@ -1357,7 +1356,8 @@ no_trimat:
 			q = NULL;
 		else {
 			q = parse = sob(parse + 1);	/* Optional field */
-			*(parse = son(parse)) = '\0';
+			parse = son(parse);
+			*parse = '\0';
 		}
 
 		working->sig = SIGHUP;
@@ -2010,7 +2010,6 @@ do_zipwork(struct zipwork_entry *zwork)
 	const char **args, *pgm_name, *pgm_path;
 	char *zresult;
 
-	command = NULL;
 	assert(zwork != NULL);
 	assert(zwork->zw_conf != NULL);
 	assert(zwork->zw_conf->compress > COMPRESS_NONE);
@@ -2114,8 +2113,7 @@ do_zipwork(struct zipwork_entry *zwork)
 	change_attrs(zresult, zwork->zw_conf);
 
 out:
-	if (command != NULL)
-		sbuf_delete(command);
+	sbuf_delete(command);
 	free(args);
 	free(zresult);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201902221531.x1MFVpd4046080>