Fix some tail bugs by mastojun · Pull Request #1 · coreutils/coreutils

mastojun · 2015-02-04T06:20:56Z

run below script (using inotify)

let count=0; echo $count>>a.log; tail -F a.log & while true; do mv a.log a.log.$count; let count=count+1; echo $count>>a.log; done

When the number of generated file is over the "/proc/sys/fs/inotify/max_user_watches", tail can't watch new a.log file.

mastojun · 2015-02-04T06:26:44Z

This commit fix reported bug

run below script (using inotify) ``` let count=0; echo $count>>a.log; tail -F a.log & while true; do mv a.log a.log.$count; let count=count+1; echo $count>>a.log; done ``` When the number of generated file is over the "/proc/sys/fs/inotify/max_user_watches", tail can't watch new a.log file.

mastojun · 2015-02-04T12:51:59Z

my test scipt is..

#!/bin/sh
prevMaxWatch=`sysctl -n fs.inotify.max_user_watches`
sudo sysctl -w fs.inotify.max_user_watches=10
d=rotatetest
mkdir $d
cd $d
timeout 20 ../tail -F k > out 2> /dev/null &

for i in $(seq 100); do
        echo ok_$i > k;
        sleep 0.01;
        env mv k k.$i
        grep ok out > /dev/null && ok=. || ok=X;
        printf $ok;
        /bin/cat /dev/null > out
done

sudo sysctl -w fs.inotify.max_user_watches=$prevMaxWatch
rm -rf $d

prev version stop ok_8 ~ ok_10. new version will be finish.
but this scipt require root authority. and if stop in running this script, max_user_watches is modified. So I don't add to test case.

pixelb · 2015-02-04T13:19:53Z

OK great stuff. Looking at this now.

try this .. :> k tail --follow=description k & echo ok >> k mv k m echo no >> m "no" is not show. when use inotify, do not call inotify_rm_watch when IN_MOVE_SELF event occur.

mastojun · 2015-02-04T14:55:25Z

I found another bug. see 0824fcc.

if no exist file and created after run tail, work well. (used old tail_forever function)

mkdir test
cd test
prev=0
../tail --follow=descriptor k.0 &
:>k.0
sleep 2;
for i in $(seq 100); do
        env mv k.$prev k.$i;
        echo ok_$i >> k.$i;
        sleep 1;
        prev=$i
done

but exist file was wrong (use tail_forever_inotify)

mkdir test
cd test
prev=0
:>k.0
../tail --follow=descriptor k.0 &
for i in $(seq 100); do
        env mv k.$prev k.$i;
        echo ok_$i >> k.$i;
        sleep 1;
        prev=$i
done

The -fsanitize=address run associated with v8.22-75-gf940fec failed to check make-prime-list, as src/primes.h is not regenerated with `make clean`. Running with -fsanitize=address indicates a read 1 byte beyond the allocated buffer. $ rm src/make-prime-list.o $ make AM_CFLAGS=-fsanitize=address src/make-prime-list $ src/make-prime-list 5000 ================================================================= ==13913==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61e00000fa43 at pc 0x4016f5 bp 0x7fff9d9840e0 sp 0x7fff9d9840d0 READ of size 1 at 0x61e00000fa43 thread T0 #0 0x4016f4 in main src/make-prime-list.c:214 #1 0x7f98892c5fdf in __libc_start_main (/lib64/libc.so.6+0x1ffdf) #2 0x401774 (src/make-prime-list+0x401774) 0x61e00000fa43 is located 0 bytes to the right of 2499-byte region [0x61e00000f080,0x61e00000fa43) allocated by thread T0 here: #0 0x7f98896ba7b7 in malloc (/lib64/libasan.so.1+0x577b7) #1 0x400f3f in xalloc src/make-prime-list.c:163 #2 0x400f3f in main src/make-prime-list.c:198 SUMMARY: AddressSanitizer: heap-buffer-overflow src/make-prime-list.c:214 main Shadow bytes around the buggy address: 0x0c3c7fff9ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c3c7fff9f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c3c7fff9f10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c3c7fff9f20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c3c7fff9f30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c3c7fff9f40: 00 00 00 00 00 00 00 00[03]fa fa fa fa fa fa fa 0x0c3c7fff9f50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c3c7fff9f60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c3c7fff9f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c3c7fff9f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c3c7fff9f90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa ... ==13913==ABORTING * src/make-prime-list.c (main): Bounds check the incremented index, before using to access the buffer. Fixes http://bugs.gnu.org/19784

pixelb · 2015-04-13T08:49:44Z

Both issues here should be addressed by: commit d313a0b and commit 235d52c

This issue was identified by running the test suite with http://code.google.com/p/address-sanitizer/ which is included in GCC 4.8 and enabled with -fsanitize=address This was checked on Fedora 20 with GCC 4.8 as follows: $ yum install libasan # http://bugzilla.redhat.com/991003 $ rm -f src/ptx.o $ make check AM_CFLAGS='-fsanitize=address' SUBDIRS=. VERBOSE=yes $ failure identified in tests/test-suite.log To see this particular failure triggered with multiple files: $ src/ptx <(echo a) <(echo a) 2>&1 | asan_symbolize.py -d ================================================================= ==32178==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000e74f at pc 0x435442 bp 0x7fffe8a1b290 sp 0x7fffe8a1b288 READ of size 1 at 0x60200000e74f thread T0 #0 0x435441 in define_all_fields coreutils/src/ptx.c:1425 coreutils#1 0x7fa206d31d64 in __libc_start_main ??:? coreutils#2 0x42f77c in _start ??:? 0x60200000e74f is located 1 bytes to the left of 3-byte region [0x60200000e750,0x60200000e753) allocated by thread T0 here: #0 0x421809 in realloc ??:? coreutils#1 0x439b4e in fread_file coreutils/lib/read-file.c:97 Shadow bytes around the buggy address: 0x0c047fff9c90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9cb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9cc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd =>0x0c047fff9ce0: fa fa 03 fa fa fa fd fd fa[fa]03 fa fa fa 00 00 0x0c047fff9cf0: fa fa 04 fa fa fa 04 fa fa fa fd fa fa fa fd fa 0x0c047fff9d00: fa fa 00 fa fa fa fd fa fa fa 00 fa fa fa 00 fa 0x0c047fff9d10: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x0c047fff9d20: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa 0x0c047fff9d30: fa fa fd fa fa fa 00 fa fa fa 00 fa fa fa 00 fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==32178==ABORTING The initial report and high level analysis were from Jim Meyering... "The underlying problem is that swallow_file_in_memory() is setting the contents of the global text_buffer for the first file, then updating it (clobbering old value) for the second file. Yet, some pointers to the initial buffer have been squirreled away and later, one of them (keyafter) is presumed to point into the new "text_buffer", which it does not. The subsequent SKIP_WHITE_BACKWARDS use backs up "cursor" and goes out of bounds." * src/ptx.c (text_buffers): Maintain references for the limits of each buffer corresponding to each file, rather than just the last processed. (struct OCCURS): Add a member to map back to the corresponding file. Note normally this could be computed from the "reference" member rather than needing the extra storage, however this is not possible when in --references mode. (find_occurs_in_text): Reference the array rather than a single entry. (define_all_fields): Likewise. Also avoid computing the file index since this is now stored directly. (main): Update text_buffers[] array rather than a single text_buffer. * tests/misc/ptx-overrun.sh: Even though this issue is already triggered with AddressSanitizer, add a new case to demonstrate the whitespace trimming issue, and to trigger without AddressSanitizer. Fixes https://bugs.gnu.org/16171

This was detected in about 25% of runs with gcc -fsanitize=address ERROR: AddressSanitizer: global-buffer-overflow on address ... READ of size 4 at 0x000000416628 thread T0 #0 0x40479f in genpattern src/shred.c:782 #1 0x4050d9 in do_wipefd src/shred.c:921 #2 0x406203 in wipefile src/shred.c:1175 #3 0x406b84 in main src/shred.c:1316 #4 0x7f3454a1ef9f in __libc_start_main (/lib64/libc.so.6+0x1ff9f) #5 0x4025d8 (/tmp/coreutils-8.23/src/shred+0x4025d8) 0x000000416628 is located 56 bytes to the left of global variable '*.LC49' from 'src/shred.c' (0x416660) of size 17 0x000000416628 is located 12 bytes to the right of global variable 'patterns' from 'src/shred.c' (0x416540) of size 220 SUMMARY: AddressSanitizer: global-buffer-overflow src/shred.c:782 * src/shred.c (gen_patterns): Restrict pattern selection to the K available, which regressed due to v5.92-1462-g65533e1. * tests/misc/shred-passes.sh: Add a deterministic test case. * NEWS: Mention the bug fix. Fixes http://bugs.gnu.org/20998

ASAN reported this error for: split -n2/3 /dev/null ERROR: AddressSanitizer: negative-size-param: (size=-1) #0 0x7f0d4c36951d in __asan_memmove (/lib64/libasan.so.2+0x8d51d) #1 0x404e06 in memmove /usr/include/bits/string3.h:59 #2 0x404e06 in bytes_chunk_extract src/split.c:988 #3 0x404e06 in main src/split.c:1626 Specifically there would be invalid memory access and subsequent processing if the chunk to be extracted was beyond the initial amount read from file (which is currently capped at 128KiB). This issue is not in a released version, only being introduced in commit v8.25-4-g62e7af0 * src/split.c (bytes_chunk_extract): The initial_read != SIZE_MAX should have been combined with && rather than ||, but also this condition is always true in this function so remove entirely. * tests/split/b-chunk.sh: Add a test case. Fixes http://bugs.gnu.org/25003

This was detected with: echo a > a; pr "-S$(printf "\t\t\t")" a -m a > /dev/null Resulting in ASAN triggering: ==================================================== ERROR: AddressSanitizer: global-buffer-overflow READ of size 1 at 0x00000041b622 thread T0 #0 0x40506a in print_sep_string ../src/pr.c:2241 #1 0x407ec4 in read_line ../src/pr.c:2493 #2 0x40985c in print_page ../src/pr.c:1802 #3 0x40985c in print_files ../src/pr.c:1618 #4 0x4036e0 in main ../src/pr.c:1136 * src/pr.c (init_parameters): Ensure we only override the specified separator when it's a single tab, thus matching the calculated separator length. * tests/pr/pr-tests.pl: Add a test case. * NEWS: Mention the fix.

* src/shred.c (fillpattern): Fix the "off by one" issue when testing whether we have enough space to copy the already written portion of the buffer to the remainder of the buffer. Specifically for buffer sizes that are (3*(2^x))+1, i.e. 7,13,... we both use an uninitialized byte and invoke undefined behavior in memcpy() operation on overlapping memory regions. * tests/misc/shred-passes.sh: Add an invocation that will trigger either valgrind UMR, or ASAN like: ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges #1 0x403065 in fillpattern src/shred.c:293 A direct test is awkward due to the random writes surrounding the problematic pattern writes. Fixes http://bugs.gnu.org/26545

Avoid "Unicode character U+coreutils#1 not supported, sorry" error when converting from texi to dvi or pdf. * doc/coreutils.texi (tr invocation): Avoid the @U{XXXX} texi representation, as even though info and html can represent these characters directly, there are conversion errors for pdf and dvi. Instead use the more abstract shell $'\uXXXX' representation.

fix stop trace --follow=description to rename exist file using inotify.

0824fcc

try this .. :> k tail --follow=description k & echo ok >> k mv k m echo no >> m "no" is not show. when use inotify, do not call inotify_rm_watch when IN_MOVE_SELF event occur.

mastojun changed the title ~~Fix watch descriptor leak.~~ Fix some tail bugs Feb 4, 2015

pixelb closed this Apr 13, 2015

CorrosiveTruths mentioned this pull request Mar 23, 2022

cp --reflink=always leaves a zero-length file when failing #56

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some tail bugs#1

Fix some tail bugs#1
mastojun wants to merge 2 commits intocoreutils:masterfrom
mastojun:fix_wd_leak

mastojun commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

pixelb commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

pixelb commented Apr 13, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mastojun commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

pixelb commented Feb 4, 2015

Uh oh!

mastojun commented Feb 4, 2015

Uh oh!

pixelb commented Apr 13, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants