Skip to content

Commit

Permalink
tail: fix tailing sysfs files where PAGE_SIZE > BUFSIZ
Browse files Browse the repository at this point in the history
* src/tail.c (file_lines): Ensure we use a buffer size >= PAGE_SIZE when
searching backwards to avoid seeking within a file,
which on sysfs files is accepted but also returns no data.
* tests/tail/tail-sysfs.sh: Add a new test.
* tests/local.mk: Reference the new test.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/67490
  • Loading branch information
dannf authored and pixelb committed Dec 1, 2023
1 parent 615167c commit 73d119f
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 15 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ GNU coreutils NEWS -*- outline -*-
numfmt options like --suffix no longer have an arbitrary 127-byte limit.
[bug introduced with numfmt in coreutils-8.21]

tail no longer mishandles input from files in /proc and /sys file systems,
on systems with a page size larger than the stdio BUFSIZ.
[This bug was present in "the beginning".]

wc no longer fails to count unprintable characters as parts of words.
[bug introduced in textutils-2.1]

Expand Down
57 changes: 42 additions & 15 deletions src/tail.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ static int nbpids = 0;
static pid_t * pids = nullptr;
static idx_t pids_alloc;

/* Used to determine the buffer size when scanning backwards in a file. */
static idx_t page_size;

/* True if we have ever read standard input. */
static bool have_read_stdin;

Expand Down Expand Up @@ -515,30 +518,49 @@ xlseek (int fd, off_t offset, int whence, char const *filename)
Return true if successful. */

static bool
file_lines (char const *pretty_filename, int fd, uintmax_t n_lines,
off_t start_pos, off_t end_pos, uintmax_t *read_pos)
file_lines (char const *pretty_filename, int fd, struct stat const *sb,
uintmax_t n_lines, off_t start_pos, off_t end_pos,
uintmax_t *read_pos)
{
char buffer[BUFSIZ];
char *buffer;
size_t bytes_read;
blksize_t bufsize = BUFSIZ;
off_t pos = end_pos;
bool ok = true;

if (n_lines == 0)
return true;

/* Be careful with files with sizes that are a multiple of the page size,
as on /proc or /sys file systems these files accept seeking to within
the file, but then return no data when read. So use a buffer that's
at least PAGE_SIZE to avoid seeking within such files.
We could also indirectly use a large enough buffer through io_blksize()
however this would be less efficient in the common case, as it would
generally pick a larger buffer size, resulting in reading more data
from the end of the file. */
affirm (S_ISREG (sb->st_mode));
if (sb->st_size % page_size == 0)
bufsize = MAX (BUFSIZ, page_size);

buffer = xmalloc (bufsize);

/* Set 'bytes_read' to the size of the last, probably partial, buffer;
0 < 'bytes_read' <= 'BUFSIZ'. */
bytes_read = (pos - start_pos) % BUFSIZ;
0 < 'bytes_read' <= 'bufsize'. */
bytes_read = (pos - start_pos) % bufsize;
if (bytes_read == 0)
bytes_read = BUFSIZ;
/* Make 'pos' a multiple of 'BUFSIZ' (0 if the file is short), so that all
bytes_read = bufsize;
/* Make 'pos' a multiple of 'bufsize' (0 if the file is short), so that all
reads will be on block boundaries, which might increase efficiency. */
pos -= bytes_read;
xlseek (fd, pos, SEEK_SET, pretty_filename);
bytes_read = safe_read (fd, buffer, bytes_read);
if (bytes_read == SAFE_READ_ERROR)
{
error (0, errno, _("error reading %s"), quoteaf (pretty_filename));
return false;
ok = false;
goto free_buffer;
}
*read_pos = pos + bytes_read;

Expand All @@ -565,7 +587,7 @@ file_lines (char const *pretty_filename, int fd, uintmax_t n_lines,
xwrite_stdout (nl + 1, bytes_read - (n + 1));
*read_pos += dump_remainder (false, pretty_filename, fd,
end_pos - (pos + bytes_read));
return true;
goto free_buffer;
}
}

Expand All @@ -577,23 +599,26 @@ file_lines (char const *pretty_filename, int fd, uintmax_t n_lines,
xlseek (fd, start_pos, SEEK_SET, pretty_filename);
*read_pos = start_pos + dump_remainder (false, pretty_filename, fd,
end_pos);
return true;
goto free_buffer;
}
pos -= BUFSIZ;
pos -= bufsize;
xlseek (fd, pos, SEEK_SET, pretty_filename);

bytes_read = safe_read (fd, buffer, BUFSIZ);
bytes_read = safe_read (fd, buffer, bufsize);
if (bytes_read == SAFE_READ_ERROR)
{
error (0, errno, _("error reading %s"), quoteaf (pretty_filename));
return false;
ok = false;
goto free_buffer;
}

*read_pos = pos + bytes_read;
}
while (bytes_read > 0);

return true;
free_buffer:
free (buffer);
return ok;
}

/* Print the last N_LINES lines from the end of the standard input,
Expand Down Expand Up @@ -1915,7 +1940,7 @@ tail_lines (char const *pretty_filename, int fd, uintmax_t n_lines,
{
*read_pos = end_pos;
if (end_pos != 0
&& ! file_lines (pretty_filename, fd, n_lines,
&& ! file_lines (pretty_filename, fd, &stats, n_lines,
start_pos, end_pos, read_pos))
return false;
}
Expand Down Expand Up @@ -2337,6 +2362,8 @@ main (int argc, char **argv)

atexit (close_stdout);

page_size = getpagesize ();

have_read_stdin = false;

count_lines = true;
Expand Down
1 change: 1 addition & 0 deletions tests/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ all_tests = \
tests/seq/seq-precision.sh \
tests/head/head.pl \
tests/head/head-elide-tail.pl \
tests/tail/tail-sysfs.sh \
tests/tail/tail-n0f.sh \
tests/ls/ls-misc.pl \
tests/date/date.pl \
Expand Down
34 changes: 34 additions & 0 deletions tests/tail/tail-sysfs.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/sh
# sysfs files have weird properties that can be influenced by page size

# Copyright 2023 Free Software Foundation, Inc.

# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.

# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.

# You should have received a copy of the GNU General Public License
# along with this program. If not, see <https://www.gnu.org/licenses/>.

. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ tail

file='/sys/kernel/profiling'

test -r "$file" || skip_ "No $file file"

cp -f "$file" exp || framework_failure_

tail -n1 "$file" > out || fail=1
compare exp out || fail=1

tail -c2 "$file" > out || fail=1
compare exp out || fail=1

Exit $fail

0 comments on commit 73d119f

Please sign in to comment.