Re: [Bug-tar] add --ignore-missing to ignore missing input files

[Paul Eggert]

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