[Bug-cpio] chmod vs fchmod

[Mike Frysinger]

redhat put together a patch for cpio-2.6 where it would run fchmod() on the 
output fd rather than closing the fd and doing chmod() on the filename ... 
the reason for this being the race condition between opening the file and 
chmod ...

ive attached said patch (which doesnt apply cleanly to 2.7) ... is there a 
reason for not using this method ?
-mike

pgp5Ud8ackcad.pgp
Description: PGP signature

Ripped from Fedora.

http://bugs.gentoo.org/90619

--- cpio-2.6/src/copyin.c.chmodRaceC    2005-04-25 13:19:34.079310381 +0200
+++ cpio-2.6/src/copyin.c       2005-04-25 14:09:32.514889697 +0200
@@ -389,19 +389,26 @@
          continue;
        }
 
-      if (close (out_file_des) < 0)
-       error (0, errno, "%s", d->header.c_name);
-
+      /*
+       *  Avoid race condition.
+       *  Set chown and chmod before closing the file desc.
+       *  address@hidden
+       */
+       
       /* File is now copied; set attributes.  */
       if (!no_chown_flag)
-       if ((chown (d->header.c_name,
+       if ((fchown (out_file_des,
                    set_owner_flag ? set_owner : d->header.c_uid,
               set_group_flag ? set_group : d->header.c_gid) < 0)
            && errno != EPERM)
          error (0, errno, "%s", d->header.c_name);
       /* chown may have turned off some permissions we wanted. */
-      if (chmod (d->header.c_name, (int) d->header.c_mode) < 0)
+      if (fchmod (out_file_des, (int) d->header.c_mode) < 0)
+       error (0, errno, "%s", d->header.c_name);
+
+      if (close (out_file_des) < 0)
        error (0, errno, "%s", d->header.c_name);
+
       if (retain_time_flag)
        {
          times.actime = times.modtime = d->header.c_mtime;
@@ -557,6 +564,25 @@
       write (out_file_des, "", 1);
       delayed_seek_count = 0;
     }
+    
+  /*
+   *  Avoid race condition.
+   *  Set chown and chmod before closing the file desc.
+   *  address@hidden
+   */
+   
+  /* File is now copied; set attributes.  */
+  if (!no_chown_flag)
+    if ((fchown (out_file_des,
+               set_owner_flag ? set_owner : file_hdr->c_uid,
+          set_group_flag ? set_group : file_hdr->c_gid) < 0)
+       && errno != EPERM)
+      error (0, errno, "%s", file_hdr->c_name);
+  
+  /* chown may have turned off some permissions we wanted. */
+  if (fchmod (out_file_des, (int) file_hdr->c_mode) < 0)
+    error (0, errno, "%s", file_hdr->c_name);
+     
   if (close (out_file_des) < 0)
     error (0, errno, "%s", file_hdr->c_name);
 
@@ -567,18 +593,6 @@
               file_hdr->c_name, crc, file_hdr->c_chksum);
     }
 
-  /* File is now copied; set attributes.  */
-  if (!no_chown_flag)
-    if ((chown (file_hdr->c_name,
-               set_owner_flag ? set_owner : file_hdr->c_uid,
-          set_group_flag ? set_group : file_hdr->c_gid) < 0)
-       && errno != EPERM)
-      error (0, errno, "%s", file_hdr->c_name);
-  
-  /* chown may have turned off some permissions we wanted. */
-  if (chmod (file_hdr->c_name, (int) file_hdr->c_mode) < 0)
-    error (0, errno, "%s", file_hdr->c_name);
-  
   if (retain_time_flag)
     {
       struct utimbuf times;            /* For setting file times.  */
@@ -589,7 +603,7 @@
       if (utime (file_hdr->c_name, &times) < 0)
        error (0, errno, "%s", file_hdr->c_name);
     }
-  
+    
   tape_skip_padding (in_file_des, file_hdr->c_filesize);
   if (file_hdr->c_nlink > 1
       && (archive_format == arf_newascii || archive_format == arf_crcascii) )
--- cpio-2.6/src/copypass.c.chmodRaceC  2004-09-06 14:09:04.000000000 +0200
+++ cpio-2.6/src/copypass.c     2005-04-25 14:09:38.135076926 +0200
@@ -181,19 +181,25 @@
                }
              if (close (in_file_des) < 0)
                error (0, errno, "%s", input_name.ds_string);
-             if (close (out_file_des) < 0)
-               error (0, errno, "%s", output_name.ds_string);
-
+             /*
+              *  Avoid race condition.
+              *  Set chown and chmod before closing the file desc.
+              *  address@hidden
+              */
              /* Set the attributes of the new file.  */
              if (!no_chown_flag)
-               if ((chown (output_name.ds_string,
+               if ((fchown (out_file_des,
                            set_owner_flag ? set_owner : in_file_stat.st_uid,
                      set_group_flag ? set_group : in_file_stat.st_gid) < 0)
                    && errno != EPERM)
                  error (0, errno, "%s", output_name.ds_string);
              /* chown may have turned off some permissions we wanted. */
-             if (chmod (output_name.ds_string, in_file_stat.st_mode) < 0)
+             if (fchmod (out_file_des, in_file_stat.st_mode) < 0)
+               error (0, errno, "%s", output_name.ds_string);
+               
+             if (close (out_file_des) < 0)
                error (0, errno, "%s", output_name.ds_string);
+
              if (reset_time_flag)
                {
                  times.actime = in_file_stat.st_atime;