From 5e4311a40927f241abdb2262a3b56999115a0a05 Mon Sep 17 00:00:00 2001 From: asomers Date: Tue, 28 Feb 2017 22:18:05 +0000 Subject: [PATCH] MFC r311572, r311895, r311928, r311985, r312395, r312417 r311572: Fix file descriptor leaks in cmp(1) Also, add a few test cases Reported by: Coverity CID: 271624 275338 Reviewed by: ngie MFC after: 4 weeks Sponsored by: Spectra Logic Corp Differential Revision: https://reviews.freebsd.org/D9074 r311895: Fix memory leaks during "tail -r" of an irregular file * Rewrite r_buf to use standard tail queues instead of a hand-rolled circular linked list. Free dynamic allocations when done. * Remove an optimization for the case where the file is a multiple of 128KB in size and there is a scarcity of memory. * Add ATF tests for "tail -r" and its variants. Reported by: Valgrind Reviewed by: ngie MFC after: 4 weeks Sponsored by: Spectra Logic Corp Differential Revision: https://reviews.freebsd.org/D9067 r311928: Fix build of usr.bin/tail with GCC Submitted by: pluknet Reported by: pluknet MFC after: 27 days X-MFC-with: 311895 Sponsored by: Spectra Logic Corp r311985: Fix uninitialized variable CIDs in route6d The variables in question are actually return arguments, but it's still good form to initialize them. Reported by: Coverity CID: 979679 979680 MFC after: 4 weeks Sponsored by: Spectra Logic Corp r312395: Fix several Coverity CIDs in devd CID 1362055, 1362054: File descriptor leaks during shutdown CID 1362013: Potential null-termination fail with long network device names CID 1362097: Uncaught exception during memory pressure CID 1362017, 1362016: Unchecked errors, possibly resulting in weird behavior if two devd instances start at the same time. CID 1362015: Unchecked error that will probably never fail Reported by: Coverity CID: 1362055 1362054 1362013 1362097 1362017 1362016 1362015 MFC after: 4 weeks Sponsored by: Spectra Logic Corp r312417: Fix build of devd with GCC 4.2 Reported by: olivier Pointy-hat-to: asomers MFC after: 27 days X-MFC-with: 312395 Sponsored by: Spectra Logic Corp git-svn-id: svn://svn.freebsd.org/base/stable/10@314425 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- etc/mtree/BSD.tests.dist | 2 + sbin/devd/devd.cc | 22 ++- usr.bin/cmp/special.c | 2 + usr.bin/cmp/tests/Makefile | 1 + usr.bin/cmp/tests/cmp_test2.sh | 67 +++++++++ usr.bin/tail/Makefile | 6 + usr.bin/tail/reverse.c | 144 +++++++++----------- usr.bin/tail/tests/Makefile | 7 + usr.bin/tail/tests/tail_test.sh | 233 ++++++++++++++++++++++++++++++++ usr.sbin/route6d/route6d.c | 2 + 10 files changed, 404 insertions(+), 82 deletions(-) create mode 100755 usr.bin/cmp/tests/cmp_test2.sh create mode 100644 usr.bin/tail/tests/Makefile create mode 100755 usr.bin/tail/tests/tail_test.sh diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist index 2732a0aef..4ae9665b0 100644 --- a/etc/mtree/BSD.tests.dist +++ b/etc/mtree/BSD.tests.dist @@ -608,6 +608,8 @@ regress.multitest.out .. .. + tail + .. tar .. timeout diff --git a/sbin/devd/devd.cc b/sbin/devd/devd.cc index 166721932..96ed18430 100644 --- a/sbin/devd/devd.cc +++ b/sbin/devd/devd.cc @@ -95,6 +95,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include "devd.h" /* C compatible definitions */ @@ -372,7 +373,7 @@ media::do_match(config &c) s = socket(PF_INET, SOCK_DGRAM, 0); if (s >= 0) { memset(&ifmr, 0, sizeof(ifmr)); - strncpy(ifmr.ifm_name, value.c_str(), sizeof(ifmr.ifm_name)); + strlcpy(ifmr.ifm_name, value.c_str(), sizeof(ifmr.ifm_name)); if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) >= 0 && ifmr.ifm_status & IFM_AVALID) { @@ -856,8 +857,10 @@ create_socket(const char *name, int socktype) if (::bind(fd, (struct sockaddr *) & sun, slen) < 0) err(1, "bind"); listen(fd, 4); - chown(name, 0, 0); /* XXX - root.wheel */ - chmod(name, 0666); + if (chown(name, 0, 0)) /* XXX - root.wheel */ + err(1, "chown"); + if (chmod(name, 0666)) + err(1, "chmod"); return (fd); } @@ -1043,7 +1046,13 @@ event_loop(void) buffer[rv] = '\0'; while (buffer[--rv] == '\n') buffer[rv] = '\0'; - process_event(buffer); + try { + process_event(buffer); + } + catch (std::length_error e) { + devdlog(LOG_ERR, "Dropping event %s " + "due to low memory", buffer); + } } else if (rv < 0) { if (errno != EINTR) break; @@ -1061,6 +1070,8 @@ event_loop(void) if (FD_ISSET(seqpacket_fd, &fds)) new_client(seqpacket_fd, SOCK_SEQPACKET); } + close(seqpacket_fd); + close(stream_fd); close(fd); } @@ -1203,7 +1214,8 @@ check_devd_enabled() if (val == 0) { warnx("Setting " SYSCTL " to 1000"); val = 1000; - sysctlbyname(SYSCTL, NULL, NULL, &val, sizeof(val)); + if (sysctlbyname(SYSCTL, NULL, NULL, &val, sizeof(val))) + err(1, "sysctlbyname"); } } diff --git a/usr.bin/cmp/special.c b/usr.bin/cmp/special.c index 8225185f0..edb641c8e 100644 --- a/usr.bin/cmp/special.c +++ b/usr.bin/cmp/special.c @@ -99,6 +99,8 @@ eof: if (ferror(fp1)) } else if (feof(fp2)) eofmsg(file2); + fclose(fp2); + fclose(fp1); if (dfound) exit(DIFF_EXIT); } diff --git a/usr.bin/cmp/tests/Makefile b/usr.bin/cmp/tests/Makefile index bfae46c8a..087f32f41 100644 --- a/usr.bin/cmp/tests/Makefile +++ b/usr.bin/cmp/tests/Makefile @@ -2,6 +2,7 @@ .include +ATF_TESTS_SH+= cmp_test2 NETBSD_ATF_TESTS_SH= cmp_test .include diff --git a/usr.bin/cmp/tests/cmp_test2.sh b/usr.bin/cmp/tests/cmp_test2.sh new file mode 100755 index 000000000..2221e6238 --- /dev/null +++ b/usr.bin/cmp/tests/cmp_test2.sh @@ -0,0 +1,67 @@ +# Copyright (c) 2017 Alan Somers +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS +# ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED +# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS +# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# +# $FreeBSD$ + +atf_test_case special +special_head() { + atf_set "descr" "Test cmp(1)'s handling of non-regular files" +} +special_body() { + echo 0123456789abcdef > a + echo 0123456789abcdeg > b + cat a | atf_check -s exit:0 cmp a - + cat a | atf_check -s exit:0 cmp - a + cat b | atf_check -s not-exit:0 cmp a - + cat b | atf_check -s not-exit:0 cmp - a + true +} + +atf_test_case symlink +symlink_head() { + atf_set "descr" "Test cmp(1)'s handling of symlinks" +} +symlink_body() { + echo 0123456789abcdef > a + echo 0123456789abcdeg > b + ln -s a a.lnk + ln -s b b.lnk + ln -s a a2.lnk + cp a adup + ln -s adup adup.lnk + atf_check -s exit:0 cmp a a.lnk + atf_check -s exit:0 cmp a.lnk a + atf_check -s not-exit:0 -o ignore cmp a b.lnk + atf_check -s not-exit:0 -o ignore cmp b.lnk a + atf_check -s not-exit:0 -o ignore -e ignore cmp -h a a.lnk + atf_check -s not-exit:0 -o ignore -e ignore cmp -h a.lnk a + atf_check -s exit:0 cmp -h a.lnk a2.lnk + atf_check -s not-exit:0 -o ignore -e ignore cmp -h a.lnk adup.lnk +} + +atf_init_test_cases() +{ + atf_add_test_case special + atf_add_test_case symlink +} diff --git a/usr.bin/tail/Makefile b/usr.bin/tail/Makefile index 672cbed00..a7f8526f4 100644 --- a/usr.bin/tail/Makefile +++ b/usr.bin/tail/Makefile @@ -1,7 +1,13 @@ # $FreeBSD$ # @(#)Makefile 8.1 (Berkeley) 6/6/93 +.include + PROG= tail SRCS= forward.c misc.c read.c reverse.c tail.c +.if ${MK_TESTS} != "no" +SUBDIR+= tests +.endif + .include diff --git a/usr.bin/tail/reverse.c b/usr.bin/tail/reverse.c index 87269051e..88c328a3a 100644 --- a/usr.bin/tail/reverse.c +++ b/usr.bin/tail/reverse.c @@ -40,6 +40,7 @@ static char sccsid[] = "@(#)reverse.c 8.1 (Berkeley) 6/6/93"; __FBSDID("$FreeBSD$"); #include +#include #include #include @@ -169,12 +170,12 @@ r_reg(FILE *fp, const char *fn, enum STYLE style, off_t off, struct stat *sbp) ierr(fn); } -typedef struct bf { - struct bf *next; - struct bf *prev; - int len; - char *l; -} BF; +#define BSZ (128 * 1024) +typedef struct bfelem { + TAILQ_ENTRY(bfelem) entries; + size_t len; + char l[BSZ]; +} bfelem_t; /* * r_buf -- display a non-regular file in reverse order by line. @@ -189,64 +190,44 @@ typedef struct bf { static void r_buf(FILE *fp, const char *fn) { - BF *mark, *tl, *tr; - int ch, len, llen; + struct bfelem *tl, *first = NULL; + size_t llen; char *p; - off_t enomem; + off_t enomem = 0; + TAILQ_HEAD(bfhead, bfelem) head; + + TAILQ_INIT(&head); + + while (!feof(fp)) { + size_t len; - tl = NULL; -#define BSZ (128 * 1024) - for (mark = NULL, enomem = 0;;) { /* * Allocate a new block and link it into place in a doubly * linked list. If out of memory, toss the LRU block and * keep going. */ - if (enomem || (tl = malloc(sizeof(BF))) == NULL || - (tl->l = malloc(BSZ)) == NULL) { - if (!mark) + while ((tl = malloc(sizeof(bfelem_t))) == NULL) { + first = TAILQ_FIRST(&head); + if (TAILQ_EMPTY(&head)) err(1, "malloc"); - if (enomem) - tl = tl->next; - else { - if (tl) - free(tl); - tl = mark; - } - enomem += tl->len; - } else if (mark) { - tl->next = mark; - tl->prev = mark->prev; - mark->prev->next = tl; - mark->prev = tl; - } else { - mark = tl; - mark->next = mark->prev = mark; + enomem += first->len; + TAILQ_REMOVE(&head, first, entries); + free(first); } + TAILQ_INSERT_TAIL(&head, tl, entries); /* Fill the block with input data. */ - for (p = tl->l, len = 0; - len < BSZ && (ch = getc(fp)) != EOF; ++len) - *p++ = ch; - - if (ferror(fp)) { - ierr(fn); - return; - } - - /* - * If no input data for this block and we tossed some data, - * recover it. - */ - if (!len && enomem) { - enomem -= tl->len; - tl = tl->prev; - break; + len = 0; + while ((!feof(fp)) && len < BSZ) { + p = tl->l + len; + len += fread(p, 1, BSZ - len, fp); + if (ferror(fp)) { + ierr(fn); + return; + } } tl->len = len; - if (ch == EOF) - break; } if (enomem) { @@ -255,37 +236,46 @@ r_buf(FILE *fp, const char *fn) } /* - * Step through the blocks in the reverse order read. The last char - * is special, ignore whether newline or not. + * Now print the lines in reverse order + * Outline: + * Scan backward for "\n", + * print forward to the end of the buffers + * free any buffers that start after the "\n" just found + * Loop */ - for (mark = tl;;) { - for (p = tl->l + (len = tl->len) - 1, llen = 0; len--; - --p, ++llen) - if (*p == '\n') { - if (llen) { + tl = TAILQ_LAST(&head, bfhead); + first = TAILQ_FIRST(&head); + while (tl != NULL) { + struct bfelem *temp; + + for (p = tl->l + tl->len - 1, llen = 0; p >= tl->l; + --p, ++llen) { + int start = (tl == first && p == tl->l); + + if ((*p == '\n') || start) { + struct bfelem *tr; + + if (start && llen) + WR(p, llen + 1); + else if (llen) WR(p + 1, llen); - llen = 0; - } - if (tl == mark) - continue; - for (tr = tl->next; tr->len; tr = tr->next) { - WR(tr->l, tr->len); - tr->len = 0; - if (tr == mark) - break; + tr = TAILQ_NEXT(tl, entries); + llen = 0; + if (tr != NULL) { + TAILQ_FOREACH_FROM_SAFE(tr, &head, + entries, temp) { + if (tr->len) + WR(&tr->l, tr->len); + TAILQ_REMOVE(&head, tr, + entries); + free(tr); + } } } + } tl->len = llen; - if ((tl = tl->prev) == mark) - break; - } - tl = tl->next; - if (tl->len) { - WR(tl->l, tl->len); - tl->len = 0; - } - while ((tl = tl->next)->len) { - WR(tl->l, tl->len); - tl->len = 0; + tl = TAILQ_PREV(tl, bfhead, entries); } + TAILQ_REMOVE(&head, first, entries); + free(first); } diff --git a/usr.bin/tail/tests/Makefile b/usr.bin/tail/tests/Makefile new file mode 100644 index 000000000..84338e292 --- /dev/null +++ b/usr.bin/tail/tests/Makefile @@ -0,0 +1,7 @@ +# $FreeBSD$ + +PACKAGE= tests + +ATF_TESTS_SH= tail_test + +.include diff --git a/usr.bin/tail/tests/tail_test.sh b/usr.bin/tail/tests/tail_test.sh new file mode 100755 index 000000000..3e407ea3f --- /dev/null +++ b/usr.bin/tail/tests/tail_test.sh @@ -0,0 +1,233 @@ +# Copyright (c) 2016 Alan Somers +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. +# +# $FreeBSD$ + +atf_test_case empty_r +empty_r_head() +{ + atf_set "descr" "Reverse an empty file" +} +empty_r_body() +{ + touch infile expectfile + tail -r infile > outfile + tail -r < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case file_r +file_r_head() +{ + atf_set "descr" "Reverse a file" +} +file_r_body() +{ + cat > infile < expectfile << HERE +This is the third line +This is the second line +This is the first line +HERE + tail -r infile > outfile + tail -r < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case file_rn2 +file_rn2_head() +{ + atf_set "descr" "Reverse the last two lines of a file" +} +file_rn2_body() +{ + cat > infile < expectfile << HERE +This is the third line +This is the second line +HERE + tail -rn2 infile > outfile + tail -rn2 < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case file_rc28 +file_rc28_head() +{ + atf_set "descr" "Reverse a file and display the last 28 characters" +} +file_rc28_body() +{ + cat > infile < expectfile << HERE +This is the third line +line +HERE + tail -rc28 infile > outfile + tail -rc28 < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case longfile_r +longfile_r_head() +{ + atf_set "descr" "Reverse a long file" +} +longfile_r_body() +{ + jot -w "%0511d" 1030 0 > infile + jot -w "%0511d" 1030 1029 0 -1 > expectfile + tail -r infile > outfile + tail -r < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case longfile_r_enomem +longfile_r_enomem_head() +{ + atf_set "descr" "Reverse a file that's too long to store in RAM" +} +longfile_r_enomem_body() +{ + # When we reverse a file that's too long for RAM, tail should drop the + # first part and just print what it can. We'll check that the last + # part is ok + { + ulimit -v 32768 || atf_skip "Can't adjust ulimit" + jot -w "%01023d" 32768 0 | tail -r > outfile ; + } + if [ "$?" -ne 1 ]; then + atf_skip "Didn't get ENOMEM. Adjust test parameters" + fi + # We don't know how much of the input we dropped. So just check that + # the first ten lines of tail's output are the same as the last ten of + # the input + jot -w "%01023d" 10 32767 0 -1 > expectfile + head -n 10 outfile > outtrunc + diff expectfile outtrunc + atf_check cmp expectfile outtrunc +} + +atf_test_case longfile_r_longlines +longfile_r_longlines_head() +{ + atf_set "descr" "Reverse a long file with extremely long lines" +} +longfile_r_longlines_body() +{ + jot -s " " -w "%07d" 18000 0 > infile + jot -s " " -w "%07d" 18000 18000 >> infile + jot -s " " -w "%07d" 18000 36000 >> infile + jot -s " " -w "%07d" 18000 36000 > expectfile + jot -s " " -w "%07d" 18000 18000 >> expectfile + jot -s " " -w "%07d" 18000 0 >> expectfile + tail -r infile > outfile + tail -r < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case longfile_rc135782 +longfile_rc135782_head() +{ + atf_set "descr" "Reverse a long file and print the last 135,782 bytes" +} +longfile_rc135782_body() +{ + jot -w "%063d" 9000 0 > infile + jot -w "%063d" 2121 8999 0 -1 > expectfile + echo "0000000000000000000000000000000006878" >> expectfile + tail -rc135782 infile > outfile + tail -rc135782 < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case longfile_rc145782_longlines +longfile_rc145782_longlines_head() +{ + atf_set "descr" "Reverse a long file with extremely long lines and print the last 145,782 bytes" +} +longfile_rc145782_longlines_body() +{ + jot -s " " -w "%07d" 18000 0 > infile + jot -s " " -w "%07d" 18000 18000 >> infile + jot -s " " -w "%07d" 18000 36000 >> infile + jot -s " " -w "%07d" 18000 36000 > expectfile + echo -n "35777 " >> expectfile + jot -s " " -w "%07d" 222 35778 >> expectfile + tail -rc145782 infile > outfile + tail -rc145782 < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + +atf_test_case longfile_rn2500 +longfile_rn2500_head() +{ + atf_set "descr" "Reverse a long file and print the last 2,500 lines" +} +longfile_rn2500_body() +{ + jot -w "%063d" 9000 0 > infile + jot -w "%063d" 2500 8999 0 -1 > expectfile + tail -rn2500 infile > outfile + tail -rn2500 < infile > outpipe + atf_check cmp expectfile outfile + atf_check cmp expectfile outpipe +} + + +atf_init_test_cases() +{ + atf_add_test_case empty_r + atf_add_test_case file_r + atf_add_test_case file_rc28 + atf_add_test_case file_rn2 + # The longfile tests are designed to exercise behavior in r_buf(), + # which operates on 128KB blocks + atf_add_test_case longfile_r + atf_add_test_case longfile_r_enomem + atf_add_test_case longfile_r_longlines + atf_add_test_case longfile_rc135782 + atf_add_test_case longfile_rc145782_longlines + atf_add_test_case longfile_rn2500 +} diff --git a/usr.sbin/route6d/route6d.c b/usr.sbin/route6d/route6d.c index 926d82a1f..90156422c 100644 --- a/usr.sbin/route6d/route6d.c +++ b/usr.sbin/route6d/route6d.c @@ -1063,6 +1063,7 @@ sendpacket(struct sockaddr_in6 *sin6, int len) iov[0].iov_len = len; m.msg_iov = iov; m.msg_iovlen = 1; + m.msg_flags = 0; if (!idx) { m.msg_control = NULL; m.msg_controllen = 0; @@ -1127,6 +1128,7 @@ riprecv(void) cm = (struct cmsghdr *)cmsgbuf; m.msg_control = (caddr_t)cm; m.msg_controllen = sizeof(cmsgbuf); + m.msg_flags = 0; if ((len = recvmsg(ripsock, &m, 0)) < 0) { fatal("recvmsg"); /*NOTREACHED*/ -- 2.42.0