On 04/24/2012 02:58 PM, Nathan Stratton Treadway wrote: > > * Using "find" is an easy way to generate lots of files to be > processed and thus increase the chances that some will be deleted > while tar is running... but it seems like it could just as easily > happen using shell wildcards directly on the tar command line.
Thanks for your thoughtful comments. Of course you are right that there is a problem if a backup script uses a wildcard, and if the parent directory is writeable to attackers, but these are normally pretty rare events, and the answer (as documented in the tar manual, or should be) is "don't do that". In contrast, the cases I'm worried about are the common cases, where the attacker can write some subdirectory of a file system that's being backed up by root. The cases I'm worried about do not involve using 'find' and generating a boatload of file names and shipping them off to tar -- my guess is that the 'find' scenarios have security holes, but that's a different matter. > * I think Stefan's point is that in this case he _expects_ ENOENT to > happen sometimes, but he doesn't expect those other conditions to > occur. But they *can* occur. I don't think we should reason merely by what looking at what Stefan happens to have observed. We should think about and understand the underlying problem and fix the problem in general, rather than just patch one symptom. The general idea here, as I understand it, is ignoring problems that can occur merely because tar is backing up a live file system, while not ignoring I/O errors that happen for other reasons. Tar already attempts to do that, by means of its file_removed_diag function, and any fix along these lines should be using that function rather than causing all 'stat' failures with errno==ENOENT to not be an error (which is clearly the wrong thing to do). In other words, there shouldn't be a need for a new option at all; the functionality is already there, it's just that it's apparently buggy in some cases (cases that haven't been described well). Incidentally, my brief and unsuccessful attempt to understand the problem led me to the following unrelated fix, which I just now pushed: >From 6f6822db3262f20e47a776e826a61f6145a182fc Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Tue, 24 Apr 2012 22:05:43 -0700 Subject: [PATCH] * src/compare.c (diff_dumpdir): Omit useless 'stat'. --- src/compare.c | 9 --------- 1 files changed, 0 insertions(+), 9 deletions(-) diff --git a/src/compare.c b/src/compare.c index ea41cb3..7c514b9 100644 --- a/src/compare.c +++ b/src/compare.c @@ -362,15 +362,6 @@ static void diff_dumpdir (struct tar_stat_info *dir) { const char *dumpdir_buffer; - struct stat stat_data; - - if (deref_stat (dir->file_name, &stat_data) != 0) - { - if (errno == ENOENT) - stat_warn (dir->file_name); - else - stat_error (dir->file_name); - } if (dir->fd == 0) { -- 1.7.6.5
