Browse Source

tar: don't assume O_NONBLOCK is benign on regular files

On Data Migration Facility (DMF), High Peformance Storage System (HPSS),
and presumably other file systems based on hierarchical storage, opening
a regular file with O_NONBLOCK can cause later reads to fail with
errno == EAGAIN.  We need the O_NONBLOCK to avoid some security races.
Work around the problem by using fcntl to clear the O_NONBLOCK
flag if I/O fails with that errno value.
Problem reported by Vitezslav Cizek in
<http://lists.gnu.org/archive/html/bug-tar/2012-01/msg00000.html>.
* src/common.h (blocking_read, blocking_write): New decls.
* src/misc.c (blocking_read, blocking_write): New functions.
* src/compare.c (process_rawdata):
* src/create.c (dump_regular_file):
* src/extract.c (extract_file):
* src/sparse.c (sparse_scan_file, sparse_extract_region):
Paul Eggert 13 years ago
parent
commit
03858cf583
6 changed files with 64 additions and 10 deletions
  1. 3 0
      src/common.h
  2. 4 4
      src/compare.c
  3. 1 1
      src/create.c
  4. 2 2
      src/extract.c
  5. 51 0
      src/misc.c
  6. 3 3
      src/sparse.c

+ 3 - 0
src/common.h

@@ -621,6 +621,9 @@ void undo_last_backup (void);
 
 int deref_stat (char const *name, struct stat *buf);
 
+size_t blocking_read (int fd, void *buf, size_t count);
+size_t blocking_write (int fd, void const *buf, size_t count);
+
 extern int chdir_current;
 extern int chdir_fd;
 int chdir_arg (char const *dir);

+ 4 - 4
src/compare.c

@@ -80,7 +80,7 @@ process_noop (size_t size __attribute__ ((unused)),
 static int
 process_rawdata (size_t bytes, char *buffer)
 {
-  size_t status = safe_read (diff_handle, diff_buffer, bytes);
+  size_t status = blocking_read (diff_handle, diff_buffer, bytes);
 
   if (status != bytes)
     {
@@ -390,7 +390,7 @@ diff_dumpdir (struct tar_stat_info *dir)
 	  file_removed_diag (dir->orig_file_name, false, diag);
 	  return;
 	}
-    }      
+    }
   dumpdir_buffer = directory_contents (scan_directory (dir));
 
   if (dumpdir_buffer)
@@ -462,7 +462,7 @@ diff_multivol (void)
 void
 diff_archive (void)
 {
-  
+
   set_next_block_after (current_header);
 
   /* Print the block from current_header and current_stat_info.  */
@@ -547,7 +547,7 @@ verify_volume (void)
 	  _("Verification may fail to locate original files.")));
 
   clear_directory_table ();
-  
+
   if (!diff_buffer)
     diff_init ();
 

+ 1 - 1
src/create.c

@@ -1051,7 +1051,7 @@ dump_regular_file (int fd, struct tar_stat_info *st)
 	    memset (blk->buffer + size_left, 0, BLOCKSIZE - count);
 	}
 
-      count = (fd <= 0) ? bufsize : safe_read (fd, blk->buffer, bufsize);
+      count = (fd <= 0) ? bufsize : blocking_read (fd, blk->buffer, bufsize);
       if (count == SAFE_READ_ERROR)
 	{
 	  read_diag_details (st->orig_file_name,

+ 2 - 2
src/extract.c

@@ -649,7 +649,7 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
 
 	case KEEP_OLD_FILES:
 	  return RECOVER_NO;
-	  
+
 	case KEEP_NEWER_FILES:
 	  if (file_newer_p (file_name, stp, &current_stat_info))
 	    break;
@@ -998,7 +998,7 @@ extract_file (char *file_name, int typeflag)
 	if (written > size)
 	  written = size;
 	errno = 0;
-	count = full_write (fd, data_block->buffer, written);
+	count = blocking_write (fd, data_block->buffer, written);
 	size -= written;
 
 	set_next_block_after ((union block *)

+ 51 - 0
src/misc.c

@@ -616,6 +616,57 @@ deref_stat (char const *name, struct stat *buf)
   return fstatat (chdir_fd, name, buf, fstatat_flags);
 }
 
+/* Read from FD into the buffer BUF with COUNT bytes.  Attempt to fill
+   BUF.  Wait until input is available; this matters because files are
+   opened O_NONBLOCK for security reasons, and on some file systems
+   this can cause read to fail with errno == EAGAIN.  Return the
+   actual number of bytes read, zero for EOF, or
+   SAFE_READ_ERROR upon error.  */
+size_t
+blocking_read (int fd, void *buf, size_t count)
+{
+  size_t bytes = safe_read (fd, buf, count);
+
+#if defined F_SETFL && O_NONBLOCK
+  if (bytes == SAFE_READ_ERROR && errno == EAGAIN)
+    {
+      int flags = fcntl (fd, F_GETFL);
+      if (0 <= flags && flags & O_NONBLOCK
+	  && fcntl (fd, F_SETFL, flags & ~O_NONBLOCK) != -1)
+	bytes = safe_read (fd, buf, count);
+    }
+#endif
+
+  return bytes;
+}
+
+/* Write to FD from the buffer BUF with COUNT bytes.  Do a full write.
+   Wait until an output buffer is available; this matters because
+   files are opened O_NONBLOCK for security reasons, and on some file
+   systems this can cause write to fail with errno == EAGAIN.  Return
+   the actual number of bytes written, setting errno if that is less
+   than COUNT.  */
+size_t
+blocking_write (int fd, void const *buf, size_t count)
+{
+  size_t bytes = full_write (fd, buf, count);
+
+#if defined F_SETFL && O_NONBLOCK
+  if (bytes < count && errno == EAGAIN)
+    {
+      int flags = fcntl (fd, F_GETFL);
+      if (0 <= flags && flags & O_NONBLOCK
+	  && fcntl (fd, F_SETFL, flags & ~O_NONBLOCK) != -1)
+	{
+	  char const *buffer = buf;
+	  bytes += full_write (fd, buffer + bytes, count - bytes);
+	}
+    }
+#endif
+
+  return bytes;
+}
+
 /* Set FD's (i.e., assuming the working directory is PARENTFD, FILE's)
    access time to ATIME.  */
 int

+ 3 - 3
src/sparse.c

@@ -230,7 +230,7 @@ sparse_scan_file (struct tar_sparse_file *file)
       if (!tar_sparse_scan (file, scan_begin, NULL))
 	return false;
 
-      while ((count = safe_read (fd, buffer, sizeof buffer)) != 0
+      while ((count = blocking_read (fd, buffer, sizeof buffer)) != 0
 	     && count != SAFE_READ_ERROR)
 	{
 	  /* Analyze the block.  */
@@ -360,7 +360,7 @@ sparse_extract_region (struct tar_sparse_file *file, size_t i)
 	  return false;
 	}
       set_next_block_after (blk);
-      count = full_write (file->fd, blk->buffer, wrbytes);
+      count = blocking_write (file->fd, blk->buffer, wrbytes);
       write_size -= count;
       file->dumped_size += count;
       mv_size_left (file->stat_info->archive_file_size - file->dumped_size);
@@ -991,7 +991,7 @@ pax_dump_header_1 (struct tar_sparse_file *file)
   off_t size = 0;
   struct sp_array *map = file->stat_info->sparse_map;
   char *save_file_name = file->stat_info->file_name;
-  
+
 #define COPY_STRING(b,dst,src) do                \
  {                                               \
    char *endp = b->buffer + BLOCKSIZE;           \