Resource fork conversion from AppleDouble v2 broken, bug #568

Converting from AppleDouble v2 to ea may corrupt the resource fork. In
some circumstances an offset calculation is wrong resulting in corrupt
resource forks after the conversion.

Ensure the resource fork AppleDouble entry offset is always set up and
we don't rely on the manual offset calculation.

Skip ADEID_RFORK in ad_copy_header() and allow copying of AppleDouble
entry that have an initial zero size in the destination adouble.

Signed-off-by: Ralph Boehme <rb@sernet.de>

diff --git a/NEWS b/NEWS
index eb10cf613100afad1caf475b15c7a2abd70f1bc9..9299f4f2b58690ec7cc1ac2e8e6b2cf0e9979421 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@ Changes in 3.1.3
 * NEW: afpd: delete empty resource forks, from FR #92
 * FIX: afpd: fix a crash when accessing ._ AppleDouble files created
        by OS X via SMB, bug #564
 * NEW: afpd: delete empty resource forks, from FR #92
 * FIX: afpd: fix a crash when accessing ._ AppleDouble files created
        by OS X via SMB, bug #564

+* FIX: afpd and dbd: Converting from AppleDouble v2 to ea may corrupt
+       the resource fork. In some circumstances an offset calculation
+       is wrong resulting in corrupt resource forks after the
+       conversion. Bug #568.

 
 Changes in 3.1.2
 ================
 
 Changes in 3.1.2
 ================

diff --git a/libatalk/adouble/ad_conv.c b/libatalk/adouble/ad_conv.c
index 2a75ec62b576bb25a6ade1a269cee52db5c98bba..43f4332baab3e4fe2a4ed464e62e2998fe2ff95d 100644 (file)
--- a/libatalk/adouble/ad_conv.c
+++ b/libatalk/adouble/ad_conv.c
@@ -152,7 +152,7 @@ static int ad_conv_v22ea_rf(const char *path, const struct stat *sp, const struc
         EC_NEG1_LOG( ad_tmplock(&adv2, ADEID_RFORK, ADLOCK_WR | ADLOCK_FILELOCK, 0, 0, 0) );
 
         /* Create a adouble:ea resource fork */
         EC_NEG1_LOG( ad_tmplock(&adv2, ADEID_RFORK, ADLOCK_WR | ADLOCK_FILELOCK, 0, 0, 0) );
 
         /* Create a adouble:ea resource fork */

-        EC_ZERO_LOG( ad_open(&adea, path, ADFLAGS_HF | ADFLAGS_RF | ADFLAGS_RDWR | ADFLAGS_CREATE, 0666) );
+        EC_ZERO_LOG( ad_open(&adea, path, ADFLAGS_RF|ADFLAGS_RDWR|ADFLAGS_CREATE|ADFLAGS_SETSHRMD, 0666) );

 
         EC_ZERO_LOG( copy_fork(ADEID_RFORK, &adea, &adv2) );
         adea.ad_rlen = adv2.ad_rlen;
 
         EC_ZERO_LOG( copy_fork(ADEID_RFORK, &adea, &adv2) );
         adea.ad_rlen = adv2.ad_rlen;

diff --git a/libatalk/adouble/ad_flush.c b/libatalk/adouble/ad_flush.c
index 6e21486302d5a69fd63cffea71788f228384647e..6a1900c72f2f718e4e334139a19dd1b4bea43369 100644 (file)
--- a/libatalk/adouble/ad_flush.c
+++ b/libatalk/adouble/ad_flush.c
@@ -217,15 +217,12 @@ int ad_copy_header(struct adouble *add, struct adouble *ads)
             continue;
 
         len = ads->ad_eid[ eid ].ade_len;
             continue;
 
         len = ads->ad_eid[ eid ].ade_len;

-        if (!len || len != add->ad_eid[ eid ].ade_len)
+        if (len == 0)

             continue;
 
         switch (eid) {
         case ADEID_COMMENT:
             continue;
 
         switch (eid) {
         case ADEID_COMMENT:

-        case ADEID_PRIVDEV:
-        case ADEID_PRIVINO:
-        case ADEID_PRIVSYN:
-        case ADEID_PRIVID:
+        case ADEID_RFORK:

             continue;
         default:
             ad_setentrylen( add, eid, len );
             continue;
         default:
             ad_setentrylen( add, eid, len );

diff --git a/libatalk/adouble/ad_open.c b/libatalk/adouble/ad_open.c
index 14f2caa4b87721ec02f34e47765b9e22e466bec9..53ec4eb97ba96c600580898ae8015a48addacf43 100644 (file)
--- a/libatalk/adouble/ad_open.c
+++ b/libatalk/adouble/ad_open.c
@@ -343,6 +343,14 @@ int ad_init_offsets(struct adouble *ad)
         eid++;
     }
 
         eid++;
     }
 

+    /*
+     * Ensure the resource fork offset is always set
+     */
+#ifndef HAVE_EAFD
+    if (ad->ad_vers == AD_VERSION_EA)
+        ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_OSX);
+#endif
+

     return 0;
 }
 
     return 0;
 }
 


@@ -795,6 +803,14 @@ static int ad_header_read_ea(const char *path, struct adouble *ad, const struct
         EC_FAIL;
     }
 
         EC_FAIL;
     }
 

+    /*
+     * Ensure the resource fork offset is always set
+     */
+#ifndef HAVE_EAFD
+    if (ad->ad_vers == AD_VERSION_EA)
+        ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_OSX);
+#endif
+

 EC_CLEANUP:
     if (ret != 0 && errno == EINVAL) {
         become_root();
 EC_CLEANUP:
     if (ret != 0 && errno == EINVAL) {
         become_root();


@@ -1809,6 +1825,12 @@ void ad_init(struct adouble *ad, const struct vol * restrict vol)
  * - we remember open fds for files because me must avoid a single close releasing fcntl locks for other
  *   fds of the same file
  *
  * - we remember open fds for files because me must avoid a single close releasing fcntl locks for other
  *   fds of the same file
  *

+ * BUGS:
+ *
+ * * on Solaris (HAVE_EAFD) ADFLAGS_RF doesn't work without
+ *   ADFLAGS_HF, because it checks whether ad_meta_fileno() is already
+ *   openend. As a workaround pass ADFLAGS_SETSHRMD.
+ *

  * @returns 0 on success, any other value indicates an error
  */
 int ad_open(struct adouble *ad, const char *path, int adflags, ...)
  * @returns 0 on success, any other value indicates an error
  */
 int ad_open(struct adouble *ad, const char *path, int adflags, ...)