Re: [Bug-tar] patches for acl, xattrs, and selinux support

[Jörg Schulz]

Bdale Garbee wrote:
> Ante Karamatic wrote:
>> Hi
>> 
>> I've created two patches, one for tar and one for tar-doc. Those
>> patches, based on RedHat's patches, add support for acl, selinux and
>> xattrs. It would be great if Debian would include those, by default.
>
> These look like generally useful stuff.  However, I've been trying hard
> to keep Debian's tar package as close to upstream bits as I can.  Are
> these patches things you'd be interested in merging into a new upstream
> release?

After trying these patches I've observed the following problems
during extraction:

1) occasionally get a warning "Cannot acl_from_text: Invalid argument"
   and the acl for this entry is not restored
2) acl default attributes for directories are not restored

The first problem comes from an error inside delay_set_stat. The
"tar_stat_info" struct pointed to by "st" should be copied on top
of the heap pointed to by "delayed_set_stat_head". But the copy
is incomplete.

The second problem comes from the fact, that during extraction
"current_stat_info.stat.st_mode" doesn't contain the information
that the current entry is a directory. But "xattrs_acls_set" decides
on this field whether to restore acl default attributes or not.

I hope the attached patch helps. It has to be applied on top of
Ante Karamatic's patch.

Jörg Schulz



diff -urN tar-1.20-1ubuntu1/debian/changelog 
tar-1.20-1ubuntu1sgbs1/debian/changelog
--- tar-1.20-1ubuntu1/debian/changelog  2009-03-09 17:07:11.000000000 +0100
+++ tar-1.20-1ubuntu1sgbs1/debian/changelog     2009-03-09 16:54:43.000000000 
+0100
@@ -1,3 +1,10 @@
+tar (1.20-1ubuntu1sgbs1) hardy; urgency=low
+
+  * fixed bug in delay_set_stat
+  * xattrs_acls_set didn't restore default attributes in directories 
+
+ -- Jörg Schulz <address@hidden>  Mon, 09 Mar 2009 16:12:38 +0100
+
 tar (1.20-1ubuntu1) jaunty; urgency=low
 
   * Added support for acl, xattrs and selinux
diff -urN tar-1.20-1ubuntu1/src/extract.c tar-1.20-1ubuntu1sgbs1/src/extract.c
--- tar-1.20-1ubuntu1/src/extract.c     2009-03-09 17:07:11.000000000 +0100
+++ tar-1.20-1ubuntu1sgbs1/src/extract.c        2009-03-09 16:54:43.000000000 
+0100
@@ -378,14 +378,20 @@
   data->cntx_name = NULL;
   assign_string (&data->cntx_name, st->cntx_name);
   if (st->acls_a_ptr)
-    data->acls_a_ptr = xmemdup(st->acls_a_ptr, st->acls_a_len);
+    {
+      data->acls_a_ptr = xstrdup(st->acls_a_ptr);
+      data->acls_a_len = st->acls_a_len;
+    }
   else
     {
       data->acls_a_ptr = NULL;
       data->acls_a_len = 0;
     }
   if (st->acls_d_ptr)
-    data->acls_d_ptr = xmemdup(st->acls_d_ptr, st->acls_d_len);
+    {
+      data->acls_d_ptr = xstrdup(st->acls_d_ptr);
+      data->acls_d_len = st->acls_d_len;
+    }
   else
     {
       data->acls_d_ptr = NULL;
@@ -689,6 +695,16 @@
   else if (typeflag == GNUTYPE_DUMPDIR)
     skip_member ();
 
+  // S_IFDIR is never set in current_stat_info.stat.st_mode
+  // (it would be if it was stored in the mode field of the tar-file 
+  // but I don't know if this would break the tar-specs or make the
+  // tar-file not portable across different implementations.
+  // so maybe this is a hack)
+  // but xattr_acls_set needs to know about it, in order to restore
+  // default attributes for directories
+
+  current_stat_info.stat.st_mode |= S_IFDIR;
+
   mode = current_stat_info.stat.st_mode | (we_are_root ? 0 : MODE_WXUSR);
   if (0 < same_owner_option || current_stat_info.stat.st_mode & ~ MODE_RWX)
     mode &= S_IRWXU;