system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-10-07 00:50:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

tail: Fix heap overflow in -F case.

Browse source 
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
This commit is contained in:
Dag-Erling Smørgrav 2023-11-29 22:48:50 +01:00
parent b647615ede
commit 621f45532c
2 changed files with 38 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
usr.bin/tail/forward.c
View file

@ -272,7 +272,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 &&
@ -304,27 +304,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;
@ -334,9 +328,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, "Couldn't allocate memory for kevents.");
set_events(files);
for (;;) {
@ -410,9 +409,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) {

21
usr.bin/tail/tests/tail_test.sh
View file

@ -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