I'm not certain that the buffer overrun in 1/3 can actually be triggered, but it's worth repairing before anything else.
Also, I found the internal name, wd_table, to be inadequate, so now it's called wd_to_name. >From 66b15949df877a75a413ad40e88aa89c3c467d34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Tue, 29 Dec 2009 14:24:01 +0100 Subject: [PATCH 1/3] tail: avoid read-beyond-end-of-buffer error * src/tail.c (tail_forever_inotify): Do not use f[i] in a context where i may be larger than the largest valid index. In the final "if" clause in which we'd remove an inotify watch, we might have used f[n_files]. Use fspec instead, since it is guaranteed to be defined. --- src/tail.c | 27 +++++++++++++-------------- 1 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/tail.c b/src/tail.c index 89f37f6..2bbf5c4 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1264,7 +1264,6 @@ wd_comparator (const void *e1, const void *e2) } /* Helper function used by `tail_forever_inotify'. */ - static void check_fspec (struct File_spec *fspec, int wd, int *prev_wd) { @@ -1306,12 +1305,10 @@ check_fspec (struct File_spec *fspec, int wd, int *prev_wd) /* Tail N_FILES files forever, or until killed. Check modifications using the inotify events system. */ - static void tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, double sleep_interval) { - size_t i; unsigned int max_realloc = 3; Hash_table *wd_table; @@ -1330,6 +1327,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, /* Add an inotify watch for each watched file. If -F is specified then watch its parent directory too, in this way when they re-appear we can add them again to the watch list. */ + size_t i; for (i = 0; i < n_files; i++) { if (!f[i].ignore) @@ -1459,34 +1457,35 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (ev->len) /* event on ev->name in watched directory */ { - for (i = 0; i < n_files; i++) + size_t j; + for (j = 0; j < n_files; j++) { /* With N=hundreds of frequently-changing files, this O(N^2) process might be a problem. FIXME: use a hash table? */ - if (f[i].parent_wd == ev->wd - && STREQ (ev->name, f[i].name + f[i].basename_start)) + if (f[j].parent_wd == ev->wd + && STREQ (ev->name, f[j].name + f[j].basename_start)) break; } /* It is not a watched file. */ - if (i == n_files) + if (j == n_files) continue; /* It's fine to add the same file more than once. */ - f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask); + f[j].wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask); - if (f[i].wd < 0) + if (f[j].wd < 0) { - error (0, errno, _("cannot watch %s"), quote (f[i].name)); + error (0, errno, _("cannot watch %s"), quote (f[j].name)); continue; } - fspec = &(f[i]); + fspec = &(f[j]); if (hash_insert (wd_table, fspec) == NULL) xalloc_die (); if (follow_mode == Follow_name) - recheck (&(f[i]), false); + recheck (&(f[j]), false); } else { @@ -1508,8 +1507,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if ((ev->mask & IN_DELETE_SELF) || ((ev->mask & IN_MOVE_SELF) && follow_mode == Follow_descriptor)) { - inotify_rm_watch (wd, f[i].wd); - hash_delete (wd_table, &(f[i])); + inotify_rm_watch (wd, fspec->wd); + hash_delete (wd_table, fspec); } if (follow_mode == Follow_name) recheck (fspec, false); -- 1.6.6.301.g5794 >From 4b2b2711d9f12b991d29755810e090133510f0d2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Tue, 29 Dec 2009 14:26:05 +0100 Subject: [PATCH 2/3] tail: rename an internal variable * src/tail.c (tail_forever_inotify): s/wd_table/wd_to_name/ --- src/tail.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/tail.c b/src/tail.c index 2bbf5c4..8e6c8ac 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1310,7 +1310,9 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, double sleep_interval) { unsigned int max_realloc = 3; - Hash_table *wd_table; + + /* Map an inotify watch descriptor to the name of the file it's watching. */ + Hash_table *wd_to_name; bool found_watchable = false; bool writer_is_dead = false; @@ -1320,8 +1322,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, size_t evbuf_off = 0; size_t len = 0; - wd_table = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL); - if (! wd_table) + wd_to_name = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL); + if (! wd_to_name) xalloc_die (); /* Add an inotify watch for each watched file. If -F is specified then watch @@ -1371,7 +1373,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, continue; } - if (hash_insert (wd_table, &(f[i])) == NULL) + if (hash_insert (wd_to_name, &(f[i])) == NULL) xalloc_die (); found_watchable = true; @@ -1481,7 +1483,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, } fspec = &(f[j]); - if (hash_insert (wd_table, fspec) == NULL) + if (hash_insert (wd_to_name, fspec) == NULL) xalloc_die (); if (follow_mode == Follow_name) @@ -1491,7 +1493,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, { struct File_spec key; key.wd = ev->wd; - fspec = hash_lookup (wd_table, &key); + fspec = hash_lookup (wd_to_name, &key); } if (! fspec) @@ -1508,7 +1510,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, || ((ev->mask & IN_MOVE_SELF) && follow_mode == Follow_descriptor)) { inotify_rm_watch (wd, fspec->wd); - hash_delete (wd_table, fspec); + hash_delete (wd_to_name, fspec); } if (follow_mode == Follow_name) recheck (fspec, false); -- 1.6.6.301.g5794 >From ed86ca7c5e4e67b59ce5cb592fef075765c7893a Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Tue, 29 Dec 2009 14:31:56 +0100 Subject: [PATCH 3/3] maint: quiet "make" in doc/ * doc/Makefile.am (constants.texi): Add a use of AM_V_GEN. --- doc/Makefile.am | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 224bcd8..c151333 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -30,13 +30,12 @@ EXTRA_DIST = perm.texi getdate.texi constants.texi fdl.texi AM_MAKEINFOFLAGS = --no-split constants.texi: $(top_srcdir)/src/tail.c $(top_srcdir)/src/shred.c - LC_ALL=C \ - sed -n -e 's/^#define \(DEFAULT_MAX[_A-Z]*\) \(.*\)/@set \1 \2/p' \ - $(top_srcdir)/src/tail.c > t-$@ - LC_ALL=C \ - sed -n -e 's/.*\(DEFAULT_PASSES\)[ =]* \([0-9]*\).*/@set SHRED_\1 \2/p'\ - $(top_srcdir)/src/shred.c >> t-$@ - mv t-$@ $@ + $(AM_V_GEN)LC_ALL=C; export LC_ALL; \ + { sed -n -e 's/^#define \(DEFAULT_MAX[_A-Z]*\) \(.*\)/@set \1 \2/p' \ + $(top_srcdir)/src/tail.c && \ + sed -n -e 's/.*\(DEFAULT_PASSES\)[ =]* \([0-9]*\).*/@set SHRED_\1 \2/p'\ + $(top_srcdir)/src/shred.c; } > t-$@ \ + && mv t-$@ $@ MAINTAINERCLEANFILES = constants.texi -- 1.6.6.301.g5794
