From nobody Wed Dec 13 20:06:35 2023 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Sr6240r0Tz54FMD; Wed, 13 Dec 2023 20:06:36 +0000 (UTC) (envelope-from git@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) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Sr62372X4z4ZVP; Wed, 13 Dec 2023 20:06:35 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1702497996; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=m9R1MIMQsAinZIjn7M2aGlN/ad7PvXHzHmF2lgpjbP8=; b=w23YcO/dU04/U6YgG/osb0LlWLwjx913kOsjWzlMF2YUMF5ZkYp+rDO4O0UvjnuygmsKpH 5MjMHmEHL4NbD7NOw5C/qPjugjAlRNYsgPmf6+M0lyGQ+m7NY90RbnZctpJanvfv3EbvcN fendEZfgNJtcTLMKTcxZLe53VapM87WgivxDEaFFqbqhCIYb5qelh4J9dsKrIdIuWbY9mk 3C4AZhUH5JO9ShLmLxhKMIbIPA51jricTSRky48rXCuEIw35xH7O2ynTg+svhw1OjAaK7m 6/HzdggkrnrwIvMYCvXaFqn0YCNzYWNl9Cp7tDiVXz4CbuCKJc41VM+Cd9tZkw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1702497996; a=rsa-sha256; cv=none; b=Rh9R9XYM/GkYxeu9Tz+RSARYdkQpiHYPh2x+GKZtYPIRs9UMC9NTTukvIOWtkHBTQaVmDy iDQzGB7qT6OfqJKSsyTX9bfO758ozSvzCKRgg0G0ffDBGilIetjP+1T2PlUpRbC/J1OHho Ff8yH7BA4Btj/+1YqRky21REOOoDRrQuk78WSzzy0n86R9tNU+01mZiK/MgD7r0PhtBvIT M+oyw0ZcenlFqqf/DBXTTHtdN2mY7ouyQygn6T9A6hRWJjoW8rR4jIFSQFGbJkezRsv5Tr QpDU5hY1CjdNjy26q8U9qHvv+6kT1TDeFZOfe/160XANHVVYMsZN/J90uB/Y7Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1702497996; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=m9R1MIMQsAinZIjn7M2aGlN/ad7PvXHzHmF2lgpjbP8=; b=Wg2BGvIiHNQXq6C9BffC/X2sep7aLNDVpV3/bNEm5KsVWhXVnyINLk8tIuzBj3YOF6fVeO azaK0sf7F2XtaWRgInu6uVA0iZE0FKZ5046tB2eV5SibFGtsn24SXfoGTZKrIQaA9mOa32 xPFitGOdbXXw5/giMvl4aDCg9cIEefaqhH/cW8ouM1TWNotbUVq+mbTufAP9NRRZe6qitR CsH1wHSqQpJ+Ke1V7/lF0WQB2qXn03qV/7qGpQSdtlbRBORlBYbEXtkEErwJE5eibWqhR5 UgYiq0R2I9SNY2WR1JDRQk9TNdWW5Q1nEFA5YMoLjmw7Bd/FEtdNdS+RJZMPRA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Sr62365Ggz1C0s; Wed, 13 Dec 2023 20:06:35 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3BDK6Zqe087668; Wed, 13 Dec 2023 20:06:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3BDK6Z2M087665; Wed, 13 Dec 2023 20:06:35 GMT (envelope-from git) Date: Wed, 13 Dec 2023 20:06:35 GMT Message-Id: <202312132006.3BDK6Z2M087665@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= Subject: git: 817f2d83bd50 - stable/13 - tail: Fix heap overflow in -F case. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 817f2d83bd50e88cb85e38e551fdaa5e79116b17 Auto-Submitted: auto-generated The branch stable/13 has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=817f2d83bd50e88cb85e38e551fdaa5e79116b17 commit 817f2d83bd50e88cb85e38e551fdaa5e79116b17 Author: Dag-Erling Smørgrav AuthorDate: 2023-11-29 21:48:50 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2023-12-13 16:39:38 +0000 tail: Fix heap overflow in -F case. The number of events we track can vary over time, but we only allocate enough space for the exact number of events we are tracking when we first begin, resulting in a trivially reproducable heap overflow. Fix this by allocating enough space for the greatest possible number of events (two per file) and clean up the code a bit. Also add a test case which triggers the aforementioned heap overflow, although we don't currently have a way to detect it. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: allanjude, markj Differential Revision: https://reviews.freebsd.org/D42839 (cherry picked from commit 621f45532c5887c96b708ce232c52878d0053325) tail: Clean up error messages. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D42842 (cherry picked from commit b70e57be2cfe83ec9f410e2f317ea38aaac61a98) --- usr.bin/tail/forward.c | 39 ++++++++++++++++++--------------------- usr.bin/tail/read.c | 8 ++++---- usr.bin/tail/reverse.c | 2 +- usr.bin/tail/tail.c | 6 +++--- usr.bin/tail/tests/tail_test.sh | 21 ++++++++++++++++++++- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/usr.bin/tail/forward.c b/usr.bin/tail/forward.c index e2a3eb77aade..a0f66262608a 100644 --- a/usr.bin/tail/forward.c +++ b/usr.bin/tail/forward.c @@ -275,7 +275,7 @@ set_events(file_info_t *files) action = USE_KQUEUE; for (i = 0, file = files; i < no_files; i++, file++) { - if (! file->fp) + if (!file->fp) continue; if (fstatfs(fileno(file->fp), &sf) == 0 && @@ -307,27 +307,21 @@ set_events(file_info_t *files) void follow(file_info_t *files, enum STYLE style, off_t off) { - int active, ev_change, i, n = -1; + int active, ev_change, i, n; struct stat sb2; file_info_t *file; FILE *ftmp; struct timespec ts; /* Position each of the files */ - - file = files; active = 0; - n = 0; - for (i = 0; i < no_files; i++, file++) { - if (file->fp) { - active = 1; - n++; - if (vflag || (qflag == 0 && no_files > 1)) - printfn(file->file_name, 1); - forward(file->fp, file->file_name, style, off, &file->st); - if (Fflag && fileno(file->fp) != STDIN_FILENO) - n++; - } + for (i = 0, file = files; i < no_files; i++, file++) { + if (!file->fp) + continue; + active = 1; + if (vflag || (qflag == 0 && no_files > 1)) + printfn(file->file_name, 1); + forward(file->fp, file->file_name, style, off, &file->st); } if (!Fflag && !active) return; @@ -337,9 +331,14 @@ follow(file_info_t *files, enum STYLE style, off_t off) kq = kqueue(); if (kq < 0) err(1, "kqueue"); - ev = malloc(n * sizeof(struct kevent)); - if (! ev) - err(1, "Couldn't allocate memory for kevents."); + /* + * The number of kqueue events we track may vary over time and may + * even grow past its initial value in the -F case, but it will + * never exceed two per file, so just preallocate that. + */ + ev = malloc(no_files * 2 * sizeof(struct kevent)); + if (ev == NULL) + err(1, "failed to allocate memory for kevents"); set_events(files); for (;;) { @@ -413,9 +412,7 @@ follow(file_info_t *files, enum STYLE style, off_t off) */ do { n = kevent(kq, NULL, 0, ev, 1, Fflag ? &ts : NULL); - if (n < 0 && errno == EINTR) - continue; - if (n < 0) + if (n < 0 && errno != EINTR) err(1, "kevent"); } while (n < 0); if (n == 0) { diff --git a/usr.bin/tail/read.c b/usr.bin/tail/read.c index ff025b31d64c..67bc5750cd0b 100644 --- a/usr.bin/tail/read.c +++ b/usr.bin/tail/read.c @@ -72,7 +72,7 @@ bytes(FILE *fp, const char *fn, off_t off) char *sp; if ((sp = p = malloc(off)) == NULL) - err(1, "malloc"); + err(1, "failed to allocate memory"); for (wrap = 0, ep = p + off; (ch = getc(fp)) != EOF;) { *p = ch; @@ -146,7 +146,7 @@ lines(FILE *fp, const char *fn, off_t off) int blen, cnt, recno, wrap; if ((llines = calloc(off, sizeof(*llines))) == NULL) - err(1, "calloc"); + err(1, "failed to allocate memory"); p = sp = NULL; blen = cnt = recno = wrap = 0; rc = 0; @@ -154,7 +154,7 @@ lines(FILE *fp, const char *fn, off_t off) while ((ch = getc(fp)) != EOF) { if (++cnt > blen) { if ((sp = realloc(sp, blen += 1024)) == NULL) - err(1, "realloc"); + err(1, "failed to allocate memory"); p = sp + cnt - 1; } *p++ = ch; @@ -163,7 +163,7 @@ lines(FILE *fp, const char *fn, off_t off) llines[recno].blen = cnt + 256; if ((llines[recno].l = realloc(llines[recno].l, llines[recno].blen)) == NULL) - err(1, "realloc"); + err(1, "failed to allocate memory"); } bcopy(sp, llines[recno].l, llines[recno].len = cnt); cnt = 0; diff --git a/usr.bin/tail/reverse.c b/usr.bin/tail/reverse.c index 12231530e679..67d27d487d1c 100644 --- a/usr.bin/tail/reverse.c +++ b/usr.bin/tail/reverse.c @@ -212,7 +212,7 @@ r_buf(FILE *fp, const char *fn) while ((tl = malloc(sizeof(bfelem_t))) == NULL) { first = TAILQ_FIRST(&head); if (TAILQ_EMPTY(&head)) - err(1, "malloc"); + err(1, "failed to allocate memory"); enomem += first->len; TAILQ_REMOVE(&head, first, entries); free(first); diff --git a/usr.bin/tail/tail.c b/usr.bin/tail/tail.c index 492a6494628d..c206b11863a5 100644 --- a/usr.bin/tail/tail.c +++ b/usr.bin/tail/tail.c @@ -173,7 +173,7 @@ main(int argc, char *argv[]) cap_rights_set(&rights, CAP_EVENT); if (caph_rights_limit(STDIN_FILENO, &rights) < 0 || caph_limit_stderr() < 0 || caph_limit_stdout() < 0) - err(1, "can't limit stdio rights"); + err(1, "unable to limit stdio rights"); fa = fileargs_init(argc, argv, O_RDONLY, 0, &rights, FA_OPEN); if (fa == NULL) @@ -213,7 +213,7 @@ main(int argc, char *argv[]) if (*argv && fflag) { files = malloc(no_files * sizeof(struct file_info)); if (files == NULL) - err(1, "Couldn't malloc space for file descriptors."); + err(1, "failed to allocate memory for file descriptors"); for (filep = files; (fn = *argv++); filep++) { filep->file_name = fn; @@ -308,7 +308,7 @@ obsolete(char *argv[]) /* Malloc space for dash, new option and argument. */ len = strlen(*argv); if ((start = p = malloc(len + 3)) == NULL) - err(1, "malloc"); + err(1, "failed to allocate memory"); *p++ = '-'; /* diff --git a/usr.bin/tail/tests/tail_test.sh b/usr.bin/tail/tests/tail_test.sh index 8123a310fe67..9c941f8a2c2f 100755 --- a/usr.bin/tail/tests/tail_test.sh +++ b/usr.bin/tail/tests/tail_test.sh @@ -329,10 +329,28 @@ follow_stdin_body() atf_check kill $pid } +atf_test_case follow_create +follow_create_head() +{ + atf_set "descr" "Verify that -F works when a file is created" +} +follow_create_body() +{ + local pid + + rm -f infile + tail -F infile > outfile & + pid=$! + seq 1 5 >infile + sleep 2 + atf_check cmp infile outfile + atf_check kill $pid +} + atf_test_case follow_rename follow_rename_head() { - atf_set "descr" "Verify that -F works" + atf_set "descr" "Verify that -F works when a file is replaced" } follow_rename_body() { @@ -424,6 +442,7 @@ atf_init_test_cases() atf_add_test_case stdin atf_add_test_case follow atf_add_test_case follow_stdin + atf_add_test_case follow_create atf_add_test_case follow_rename atf_add_test_case silent_header atf_add_test_case verbose_header