From owner-svn-src-head@freebsd.org Fri Feb 8 13:54:18 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DE41D14E13DF; Fri, 8 Feb 2019 13:54:17 +0000 (UTC) (envelope-from dab@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8200583780; Fri, 8 Feb 2019 13:54:17 +0000 (UTC) (envelope-from dab@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 7442E1A0F7; Fri, 8 Feb 2019 13:54:17 +0000 (UTC) (envelope-from dab@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x18DsHca004723; Fri, 8 Feb 2019 13:54:17 GMT (envelope-from dab@FreeBSD.org) Received: (from dab@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x18DsH7V004722; Fri, 8 Feb 2019 13:54:17 GMT (envelope-from dab@FreeBSD.org) Message-Id: <201902081354.x18DsH7V004722@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: dab set sender to dab@FreeBSD.org using -f From: David Bright Date: Fri, 8 Feb 2019 13:54:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r343906 - head/usr.sbin/newsyslog X-SVN-Group: head X-SVN-Commit-Author: dab X-SVN-Commit-Paths: head/usr.sbin/newsyslog X-SVN-Commit-Revision: 343906 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 8200583780 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.96 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-0.99)[-0.995,0]; NEURAL_HAM_LONG(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.96)[-0.961,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Feb 2019 13:54:18 -0000 Author: dab Date: Fri Feb 8 13:54:16 2019 New Revision: 343906 URL: https://svnweb.freebsd.org/changeset/base/343906 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 1007454, CID 1007453: Resource leak - The result of a strdup() was stored in a global variable and not freed before program exit. - 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 (PR158794) Reviewed by: cem, markj MFC after: 1 week Sponsored by: Dell EMC Isilon Modified: head/usr.sbin/newsyslog/newsyslog.c Modified: head/usr.sbin/newsyslog/newsyslog.c ============================================================================== --- head/usr.sbin/newsyslog/newsyslog.c Fri Feb 8 13:10:45 2019 (r343905) +++ head/usr.sbin/newsyslog/newsyslog.c Fri Feb 8 13:54:16 2019 (r343906) @@ -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); @@ -374,6 +374,8 @@ main(int argc, char **argv) while (wait(NULL) > 0 || errno == EINTR) ; + free(timefnamefmt); + free(requestor); return (0); } @@ -841,7 +843,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 +860,6 @@ get_worklist(char **files) if (defconf != NULL) free_entry(defconf); return (filelist); - /* NOTREACHED */ } /* @@ -915,7 +916,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 +1047,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 +1138,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 +1358,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 +2012,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 +2115,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); }