Re: [Bug-xorriso] bug with handling symbolic links?

[Thomas Schmitt]

Hi,

> If I understand the bigger picture correctly, I was actually wondering 
> why xorriso's SUSP might not potentially have triggered the problem at hands
> even earlier?

Quick tour:

An ECMA-119/ISO 9660 directory record (inode) can take 255
bytes.
The first 33 are occupied by attributes, then comes the
dull ECMA-119/ISO 9660 name. The rest up to 255 is usable
for SUSP entries. If this does not suffice, then SUSP entry
CE points to a block and a byte offset where the list
of SUSP entries continues.
This Continuation Area is used for tail pieces of long file
names (Rock Ridge entry NM) and of symbolic link paths
(Rock Ridge entry SL).

You need a directory with quite some spectacular list
of symbolic links in order to exceed the size of 2048 bytes.
Yours suffices. The sum is at least 5405 bytes:
  Feb 16 14:57:22 ts5 kernel: rock: corrupted directory entry. extent=85, 
offset=5305, size=100

I did not see any error messages in user space when
running ls -l on your mounted xorriso-produced ISO.
Only the /var/log/messages file is full of complaints
and the files with undigestible CE entries are missing
in the output of ls -l.

So this might have happened unnoticed somewhere.
(Only good i can advise a way to retrieve the inaccessible
 data.) 


> I have checked under
>        http://bazaar.launchpad.net/~libburnia-team/libisofs/scdbackup/changes/
> but could not yet see your changes there...

It's still in the fermentation tank. :))

Current state (only file libisofs/rockridge.c is affected):
-------------------------------------------------------------------

=== modified file 'libisofs/rockridge.c'
--- libisofs/rockridge.c        2014-04-03 19:45:19 +0000
+++ libisofs/rockridge.c        2015-02-21 14:08:30 +0000
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2007 Vreixo Formoso
  * Copyright (c) 2007 Mario Danic
- * Copyright (c) 2009 - 2012 Thomas Schmitt
+ * Copyright (c) 2009 - 2015 Thomas Schmitt
  * 
  * This file is part of the libisofs project; you can redistribute it and/or 
  * modify it under the terms of the GNU General Public License version 2 
@@ -9,6 +9,19 @@
  * See COPYING file for details.
  */
 
+/* <<< only while development of remedy */
+/* This keeps CE offsets smaller than 2048 and pads up partial blocks
+   to the next block start if offset + size is larger than 2048.
+*/ 
+#define Libisofs_B50216_bug_xorriso_joerg_meyer yes
+
+/* <<< only while development of remedy */
+/* This will split sizes larger than 2048 into independent blocks smaller
+   than 2048 which point to the next block by CE.
+   >>> not ready yet
+ # def ine Libisofs_B50216_bug_xorriso_joerg_meyer_size yes
+*/
+
 #ifdef HAVE_CONFIG_H
 #include "../config.h"
 #endif
@@ -54,6 +67,18 @@
 static
 int susp_append_ce(Ecma119Image *t, struct susp_info *susp, uint8_t *data)
 {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+    if ((susp->ce_len + data[2] - 1) / BLOCK_SIZE !=
+        susp->ce_len / BLOCK_SIZE) {
+
+        /* >>> insert CE to next block and padding field */;
+
+    }
+
+#endif /* Libisofs_B50216_bug_xorriso_joerg_meyer_size */
+
     susp->n_ce_susp_fields++;
     susp->ce_susp_fields = realloc(susp->ce_susp_fields, sizeof(void*)
                                    * susp->n_ce_susp_fields);
@@ -61,7 +86,12 @@
         return ISO_OUT_OF_MEM;
     }
     susp->ce_susp_fields[susp->n_ce_susp_fields - 1] = data;
-    susp->ce_len += data[2];
+    if (data[0] == 0) {
+        if (susp->ce_len % BLOCK_SIZE)
+            susp->ce_len += BLOCK_SIZE - (susp->ce_len % BLOCK_SIZE);
+    } else {
+        susp->ce_len += data[2];
+    }
     return ISO_SUCCESS;
 }
 
@@ -547,6 +577,20 @@
 }
 
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+static
+int susp_calc_add_to_ce(size_t *ce, int add, int flag)
+{
+    /* >>> account for inserted CE before size exceeds block size */;
+
+    *ce += add;
+    return ISO_SUCCESS;
+}
+
+#endif /* Libisofs_B50216_bug_xorriso_joerg_meyer_size */
+
+
 /*
    @param flag bit0= only account sizes in sua_free resp. ce_len
                      parameters susp and data may be NULL in this case
@@ -559,10 +603,40 @@
     int ret, done = 0, len, es_extra = 0;
     uint8_t *aapt, *cpt;
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+    size_t count = 0;
+#endif
+
     if (!t->opts->aaip_susp_1_10)
         es_extra = 5;
     if (*sua_free < num_data + es_extra || *ce_len > 0) {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+        if (es_extra > 0)
+            susp_calc_add_to_ce(ce_len, es_extra, 0);
+        done = 0;
+        for (aapt = *data; !done; aapt += aapt[2]) {
+            done = !(aapt[4] & 1);
+            len = aapt[2];
+            susp_calc_add_to_ce(ce_len, len, 0);
+            count += len;
+        }
+
+        /* <<< only while testing */
+        if (count != num_data) {
+           iso_msg_submit(t->image->id, ISO_ASSERT_FAILURE, 0,
+                          "AAIP field length sum %d <> announced AAIP size %d",
+                          count, num_data);
+           return ISO_ASSERT_FAILURE;
+        }
+
+#else /* Libisofs_B50216_bug_xorriso_joerg_meyer_size */
+
         *ce_len += num_data + es_extra;
+
+#endif /* ! Libisofs_B50216_bug_xorriso_joerg_meyer_size */
+
     } else {
         *sua_free -= num_data + es_extra;
     }
@@ -588,6 +662,7 @@
     }
 
     /* Multiple fields have to be handed over as single field copies */
+    done = 0;
     for (aapt = *data; !done; aapt += aapt[2]) {
         done = !(aapt[4] & 1);
         len = aapt[2];
@@ -730,6 +805,11 @@
 static
 int susp_add_CE(Ecma119Image *t, size_t ce_len, struct susp_info *susp)
 {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+    uint32_t block_offset, byte_offset;
+#endif
+
     uint8_t *CE = malloc(28);
     if (CE == NULL) {
         return ISO_OUT_OF_MEM;
@@ -740,8 +820,23 @@
     CE[2] = 28;
     CE[3] = 1;
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+    /* Linux fs/isofs wants byte_offset + ce_len <= BLOCK_SIZE
+     * Here the byte_offset is reduced to the minimum.
+     */
+    block_offset = susp->ce_len / BLOCK_SIZE;
+    byte_offset = susp->ce_len % BLOCK_SIZE;
+    iso_bb(&CE[4], susp->ce_block + block_offset - t->eff_partition_offset, 4);
+    iso_bb(&CE[12], byte_offset, 4);
+
+#else
+
     iso_bb(&CE[4], susp->ce_block - t->eff_partition_offset, 4);
     iso_bb(&CE[12], susp->ce_len, 4);
+
+#endif /* ! Libisofs_B50216_bug_xorriso_joerg_meyer */
+
     iso_bb(&CE[20], (uint32_t) ce_len, 4);
 
     return susp_append(t, susp, CE);
@@ -798,6 +893,28 @@
     }
 }
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+/** 
+ * A field beginning by 0 causes rrip_write_ce_fields() to pad up to the
+ * next block.
+ */
+static
+int pseudo_susp_add_PAD(Ecma119Image *t, struct susp_info *susp)
+{
+    unsigned char *pad = malloc(1);
+    int ret;
+
+    if (pad == NULL)
+        return ISO_OUT_OF_MEM;
+    pad[0] = 0;
+    ret = susp_append_ce(t, susp, pad);
+    if (ret < 0)
+        return ret;
+    return ISO_SUCCESS;
+}
+
+#endif /* Libisofs_B50216_bug_xorriso_joerg_meyer */
 
 /**
  * see doc/zisofs_format.txt : "ZF System Use Entry Format"
@@ -922,7 +1039,17 @@
 
     /* Account for field size */
     if (*sua_free < 16 || *ce_len > 0) {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+        susp_calc_add_to_ce(ce_len, 16, 0);
+
+#else
+
         *ce_len += 16;
+
+#endif
+
     } else {
         *sua_free -= 16;
     }
@@ -994,6 +1121,14 @@
     if (*ce > 0 && !(flag & 1))
         goto unannounced_ca;
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+    /* <<< in the code below:
+           account for inserted CE before size exceeds block size
+    */;
+
+#endif
+
     name = get_rr_fname(t, n->node->name);
     namelen = strlen(name);
     free(name);
@@ -1020,7 +1155,14 @@
                 of the name will always fit into the directory entry.)
         */;
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+        susp_calc_add_to_ce(ce, 5 + namelen, 0);
+
+#else
         *ce = 5 + namelen;
+#endif 
+
         *su_size = space;
     }
     if (n->type == ECMA119_SYMLINK) {
@@ -1090,7 +1232,15 @@
                              * the component can be divided between this
                              * and another SL entry
                              */
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+                            susp_calc_add_to_ce(ce, 255, 0);
+
+#else
+
                             *ce += 255; /* this SL, full */
+
+#endif
                             sl_len = 5 + (clen - fit);
                         } else {
                             /*
@@ -1098,12 +1248,35 @@
                              * any case, so we prefer to don't write 
                              * anything in this SL
                              */
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+                            /* >>> ??? is this really coordinated with
+                                       rrip_add_SL() ?
+                            */
+                            susp_calc_add_to_ce(ce, sl_len , 0);
+                            susp_calc_add_to_ce(ce, 255, 0);
+
+#else
                             *ce += sl_len + 255;
+
+#endif 
+
                             sl_len = 5 + (clen - 250) + 2;
                         }
                     } else {
                         /* case 2, create a new SL entry */
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+                        susp_calc_add_to_ce(ce, sl_len, 0);
+
+#else
+
                         *ce += sl_len;
+
+#endif 
+
                         sl_len = 5 + clen;
                     }
                 } else {
@@ -1126,7 +1299,17 @@
             /* the whole SL fits into the SUA */
             *su_size += sl_len;
         } else {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+            susp_calc_add_to_ce(ce, sl_len, 0);
+
+#else
+
             *ce += sl_len;
+
+#endif 
+
         }
 
     }
@@ -1150,6 +1333,15 @@
     /* let the expert decide where to add num_aapt */
     if (num_aapt > 0) {
         sua_free = space - *su_size;
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+        /* >>> (This is the only place where a block overflow can occur
+                if names are smaller than 256 and symlink paths are smaller
+                than 1024 */;
+
+#endif 
+
         aaip_add_AL(t, NULL, NULL, num_aapt, &sua_free, ce, 1);
         *su_size = space - sua_free;
         if (*ce > 0 && !(flag & 1))
@@ -1699,7 +1891,30 @@
         }
 
         if (ce_is_predicted) {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+            if ((info->ce_len % BLOCK_SIZE) &&
+                (info->ce_len + ce_len_pd - 1 ) / BLOCK_SIZE !=
+                info->ce_len / BLOCK_SIZE) {
+                /* Linux fs/isofs wants byte_offset + ce_len <= BLOCK_SIZE
+                 * Insert padding to shift CE offset to next block start
+                 */
+                ret = pseudo_susp_add_PAD(t, info);
+                if (ret < 0)
+                    goto add_susp_cleanup;
+            }
+
+#endif /* Libisofs_B50216_bug_xorriso_joerg_meyer */
+
             /* Add the CE entry */
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer_size
+
+            /* >>> need size of first CA <= 2048 bytes */;
+
+#endif
+
             ret = susp_add_CE(t, ce_len_pd, info);
             if (ret < 0) {
                 goto add_susp_cleanup;
@@ -1897,17 +2112,46 @@
     uint8_t *padding = NULL;
     int ret= ISO_SUCCESS;
 
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+    uint64_t written = 0, pad_size;
+#endif
+
     if (info->n_ce_susp_fields == 0) {
         goto ex;
     }
     LIBISO_ALLOC_MEM(padding, uint8_t, BLOCK_SIZE);
 
     for (i = 0; i < info->n_ce_susp_fields; i++) {
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+        if (info->ce_susp_fields[i][0] == 0) {
+            /* Pseudo field: pad up to next block boundary */
+            pad_size = BLOCK_SIZE - (written % BLOCK_SIZE);
+            if (pad_size == BLOCK_SIZE)
+    continue;
+            memset(padding, 0, pad_size);
+            ret = iso_write(t, padding, pad_size);
+            if (ret < 0)
+                goto write_ce_field_cleanup;
+            written += pad_size;
+    continue;
+        }
+
+#endif /* Libisofs_B50216_bug_xorriso_joerg_meyer */
+
         ret = iso_write(t, info->ce_susp_fields[i], 
                         info->ce_susp_fields[i][2]);
         if (ret < 0) {
             goto write_ce_field_cleanup;
         }
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+        written += info->ce_susp_fields[i][2];
+
+#endif
+
     }
 
     /* pad continuation area until block size */
@@ -1915,6 +2159,15 @@
     if (i > 0 && i < BLOCK_SIZE) {
         memset(padding, 0, i);
         ret = iso_write(t, padding, i);
+        if (ret < 0)
+            goto write_ce_field_cleanup;
+
+#ifdef Libisofs_B50216_bug_xorriso_joerg_meyer
+
+        written += i;
+
+#endif
+
     }
 
     write_ce_field_cleanup: ;

-------------------------------------------------------------------
end of current change set.

I will run a few more tests on this state and then
send you my libisofs/rockridge.c as substitute for
the file in your source installation of xorriso
resp. libisofs.


Have a nice day :)

Thomas