Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISO9660 writer segfaults on deep directories #780

Open
probonopd opened this issue Sep 10, 2016 · 31 comments
Open

ISO9660 writer segfaults on deep directories #780

probonopd opened this issue Sep 10, 2016 · 31 comments
Milestone

Comments

@probonopd
Copy link

probonopd commented Sep 10, 2016

Unfortunately https://github.com/libarchive/libarchive/wiki/FormatISO9660 has been in a bad shape for a while; please document it better.

Is it possible to write zisofs with libarchive?

.It Format iso9660 - zisofs support
The zisofs extensions permit each file to be independently compressed
using a gzip-compatible compression.
This can provide significant size savings, but requires the reading
system to have support for these extensions.
These extensions are disabled by default.
.Bl -tag -compact -width indent
.It Cm compression-level Ns = Ns number
The compression level used by the deflate compressor.
Ranges from 0 (least effort) to 9 (most effort).
Default: 6
.It Cm zisofs
Synonym for
.Cm zisofs=direct .
.It Cm zisofs=direct
Compress each file in the archive.
Unlike
.Cm zisofs=indirect ,
this is handled entirely within libarchive and does not require a
separate utility.
For best results, libarchive tests each file and will store
the file uncompressed if the compression does not actually save any space.
In particular, files under 2k will never be compressed.
Note that boot image files are never compressed.
.It Cm zisofs=indirect
Recognizes files that have already been compressed with the
.Cm mkzftree
utility and sets up the necessary file metadata so that
readers will correctly identify these as zisofs-compressed files.
.It Cm zisofs-exclude Ns = Ns Ar filename
Specifies a filename that should not be compressed when using
.Cm zisofs=direct .
This option can be provided multiple times to suppress compression
seems promising but it is unclear how to actually use this, e.g., using bsdtar, to write an ISO9660 file using libarchive and using zisofs compression.

@kientzle
Copy link
Contributor

I just expanded that Wiki page considerably. Please let us know what other information would be helpful.

@probonopd
Copy link
Author

Thank you very much for the quick response. Trying it out now.

@probonopd
Copy link
Author

probonopd commented Sep 10, 2016

It would be tremendously helpful if there could be a minimal "hello world" example like this (but actually working):

/*
 * sudo apt-get install libarchive-dev
 */

#include <archive.h>

int main(int argc, char *argv[])
{
    struct archive *iso;
    iso = archive_write_new();
    archive_write_set_format_iso9660(iso);
    // Enable RockRidge
    // Enable zisofs compression
    // Add the root directory of files (including hardlinks) 
    archive_write_free(iso);
    return 0;
}

@kientzle
Copy link
Contributor

Here's our basic example for writing an archive:

https://github.com/libarchive/libarchive/wiki/Examples#a-basic-write-example

The only pieces different for you would be setting the format differently, and setting the zisofs option with:

   archive_write_set_options(a, "iso9660:zisofs");

There are other samples in the examples directory that you may find useful.

@probonopd
Copy link
Author

Thanks, I started with minitar.c and the results look promising.

@probonopd
Copy link
Author

Is it expected that generating a large zisofs will segfault? I followed minitar.c with minimal changes to enable zisofs generation. Works when archiving a few files but not with larger amounts (e.g, 10k files with 300 MB total). Segfault happens after all the files have been shown with -v.

Here is my code.

@kientzle
Copy link
Contributor

A stack trace would certainly help to narrow down the issue.

That code is omitting a lot of error checks. In particular, you should absolutely be checking the return codes from these calls:

  archive_write_open_filename
  archive_write_data
  archive_read_close
  archive_write_close
  archive_write_disk_set_options
  archive_read_close

Note that the ISO9660 writer does a lot of work when you close the archive, so it's essential that you check the error codes from the close calls.

@probonopd
Copy link
Author

probonopd commented Sep 11, 2016

Using the method from http://stackoverflow.com/questions/77005/how-to-generate-a-stacktrace-when-my-gcc-c-app-crashes I get:

Error: signal 11:
./appimagetool(handler+0x1c)[0x401a69]
/lib/x86_64-linux-gnu/libc.so.6(+0x36ff0)[0x7f2450450ff0]
/usr/lib/x86_64-linux-gnu/libarchive.so.13(+0x6118e)[0x7f245084118e]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b779)[0x7f2450455779]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4e8)[0x7f24504554e8]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4e8)[0x7f24504554e8]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4d2)[0x7f24504554d2]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4e8)[0x7f24504554e8]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4d2)[0x7f24504554d2]
/lib/x86_64-linux-gnu/libc.so.6(+0x3b4e8)[0x7f24504554e8]

I guess that's not too helpful?

@probonopd
Copy link
Author

When I change the section to read

ret = archive_write_data(a, buff, len);
fprintf (stderr, "archive_write_data returned %i\n", ret);

then I get

a /tmp/.mount_9oacCP/Tools/QtCreator/share/qtcreator/translation/linguist_ru.qmarchive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 13765

a /tmp/.mount_9oacCP/Tools/QtCreator/share/qtcreator/translation/linguist_sk.qmarchive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 2385

a /tmp/.mount_9oacCP/Tools/QtCreator/share/qtcreator/translation/linguist_sl.qmarchive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 13751

a /tmp/.mount_9oacCP/Tools/QtCreator/share/qtcreator/translations/linguist_uk.qmarchive_write_data returned 16384
archive_write_data returned 16384
archive_write_data returned 14995

Is this normal?

@kientzle
Copy link
Contributor

kientzle commented Sep 11, 2016

That stack overflow post is discussing how to build one part of an automatic problem reporting system. It's overkill for your needs.

To get useful stack traces, you first need to recompile your app with the -g flag to enable debug information. If you are compiling and linking separately, you need to include it in both places.

The easiest way to get a useful backtrace is simply to run your program under a debugger. If you have gdb, for instance:

$ gdb <your program>
$ run <arguments to your program>

When it fails, you can then use bt to get a backtrace, up and down to switch to different function scopes, and p to examine the state of variables.

@kientzle
Copy link
Contributor

When I change the section to read

  ret = archive_write_data(a, buff, len);
  fprintf (stderr, "archive_write_data returned %i\n", ret);

then I get

  archive_write_data returned 16384
  archive_write_data returned 16384
  archive_write_data returned 13765

You should also print out the value of len, of course. If ret == len, then it's perfectly normal.

@probonopd
Copy link
Author

Thanks for the detailed instructions @kientzle helps me a lot.
Using the following procedure I can reproduce the issue with 100% certainty.

# Get and compile the tool (like minitar but for ISO9660 zisofs)
wget "https://github.com/probonopd/AppImageKit/raw/39e835752642baef05b3446f888d15feacde581b/appimagetool/appimagetool.c"
gcc -g -rdynamic appimagetool.c -larchive -o appimagetool

# Get the test data (it is a zisofs ISO9660 file)
wget -c "https://bintray.com/probono/AppImages/download_file?file_path=QtCreator-5.7.0-x86_64.AppImage" -O QtCreator-5.7.0-x86_64.AppImage

# Loop-mount
sudo mount QtCreator-5.7.0-x86_64.AppImage /mnt -o loop,ro

gdb --args ./appimagetool -cvf test.iso /mnt
run

# Program received signal SIGSEGV, Segmentation fault.
# 0x00007ffff7b9a194 in ?? () from /usr/lib/x86_64-linux-gnu/libarchive.so.13

#0  0x00007ffff7b9a194 in ?? () from /usr/lib/x86_64-linux-gnu/libarchive.so.13
#1  0x00007ffff77ae779 in msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebe08, n=2) at msort.c:83
#2  0x00007ffff77ae4e8 in msort_with_tmp (n=2, b=0x27ebe08, p=0x7fffffffdbb0) at msort.c:45
#3  msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebdf8, n=4) at msort.c:54
#4  0x00007ffff77ae4e8 in msort_with_tmp (n=4, b=0x27ebdf8, p=0x7fffffffdbb0) at msort.c:45
#5  msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebde0, n=7) at msort.c:54
#6  0x00007ffff77ae4d2 in msort_with_tmp (n=7, b=0x27ebde0, p=0x7fffffffdbb0) at msort.c:45
#7  msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebde0, n=15) at msort.c:53
#8  0x00007ffff77ae4d2 in msort_with_tmp (n=15, b=0x27ebde0, p=0x7fffffffdbb0) at msort.c:45
#9  msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebde0, n=30) at msort.c:53
#10 0x00007ffff77ae4d2 in msort_with_tmp (n=30, b=0x27ebde0, p=0x7fffffffdbb0) at msort.c:45
#11 msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebde0, n=60) at msort.c:53
#12 0x00007ffff77ae4d2 in msort_with_tmp (n=60, b=0x27ebde0, p=0x7fffffffdbb0) at msort.c:45
#13 msort_with_tmp (p=0x7fffffffdbb0, b=0x27ebde0, n=121) at msort.c:53
#14 0x00007ffff77aea2c in msort_with_tmp (n=121, b=0x27ebde0, p=0x7fffffffdbb0) at msort.c:45
#15 __GI_qsort_r (b=0x27ebde0, n=121, s=8, cmp=0x7ffff7b9a180, arg=<optimized out>) at msort.c:297
#16 0x00007ffff7b9bbdf in ?? () from /usr/lib/x86_64-linux-gnu/libarchive.so.13
#17 0x00007ffff7ba118c in ?? () from /usr/lib/x86_64-linux-gnu/libarchive.so.13
#18 0x00007ffff7b8c9ce in ?? () from /usr/lib/x86_64-linux-gnu/libarchive.so.13
#19 0x0000000000401847 in create (filename=0x7fffffffe39d "test.iso", compress=0, argv=0x7fffffffe038)
    at appimagetool.c:193
#20 0x000000000040152f in main (argc=4, argv=0x7fffffffe030) at appimagetool.c:80

So it looks to me like it has problems in msort_with_tmp, something I am not (directly) calling.

@kientzle
Copy link
Contributor

This does help a lot:

msort seems to be your C library's implementation of qsort. qsort() is used by the ISO9660 writer to sort the paths when building the directory tree. When a program calls qsort, it provides a comparison function: In libarchive's case, that comparison function is either _compare_path_table or _compare_path_table_joliet depending on whether the library is currently constructing the regular path table or the Joliet path table. So the crash is occurring in one of those comparison functions.

Those comparison functions are not so simple that I can just eyeball the problem, though.

Could you recompile a debug version of libarchive and see if you get any more detail?

@kientzle kientzle reopened this Sep 12, 2016
@probonopd
Copy link
Author

probonopd commented Sep 12, 2016

Assuming you mean CPPFLAGS=-DDEBUG CXXFLAGS="-g -O0" with "debug build", here it is:

sudo apt install -y autoconf libtool make gcc libxml2-dev libtool libtool-bin
wget https://github.com/libarchive/libarchive/archive/v3.1.2.tar.gz
tar xfv v3.1.2.tar.gz
cd libarchive-*

libtoolize --force
aclocal
autoheader
automake --force-missing --add-missing
autoconf

./configure CPPFLAGS=-DDEBUG CXXFLAGS="-g -O0" --disable-bsdtar --disable-bsdcpio --disable-posix-regex-lib --without-bz2lib  --without-lzmadec --without-iconv --without-lzo2 --without-nettle --without-openssl --without-xml2 --without-expat

make
sudo make install
libtool --finish /usr/local/lib

# Get and compile the tool (like minitar but for ISO9660 zisofs)
wget "https://github.com/probonopd/AppImageKit/raw/39e835752642baef05b3446f888d15feacde581b/appimagetool/appimagetool.c"
gcc -g -rdynamic appimagetool.c -larchive -o appimagetool

# Get the test data (it is a zisofs ISO9660 file)
wget -c "https://bintray.com/probono/AppImages/download_file?file_path=QtCreator-5.7.0-x86_64.AppImage" -O QtCreator-5.7.0-x86_64.AppImage

# Loop-mount
sudo mount QtCreator-5.7.0-x86_64.AppImage /mnt -o loop,ro

sudo rm -rf /usr/lib/x86_64-linux-gnu/libarchive*
sudo mv /usr/local/lib/libarchive.* /usr/lib/x86_64-linux-gnu/

gdb --args ./appimagetool -cvf test.iso /mnt
run
bt

# With Joliet ONLY

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b9dbd4 in _compare_path_table (v1=0x249c108, v2=<optimized out>) at libarchive/archive_write_set_format_iso9660.c:6800
6800            cmp = p1->parent->dir_number - p2->parent->dir_number;
(gdb) bt
#0  0x00007ffff7b9dbd4 in _compare_path_table (v1=0x249c108, v2=<optimized out>) at libarchive/archive_write_set_format_iso9660.c:6800
#1  0x00007ffff77b6222 in msort_with_tmp (p=0x7fffffffdc90, b=0x249c108, n=2) at msort.c:83
#2  0x00007ffff77b5ecd in msort_with_tmp (n=2, b=0x249c108, p=0x7fffffffdc90) at msort.c:45
#3  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0f8, n=4) at msort.c:54
#4  0x00007ffff77b5ecd in msort_with_tmp (n=4, b=0x249c0f8, p=0x7fffffffdc90) at msort.c:45
#5  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=7) at msort.c:54
#6  0x00007ffff77b5eb7 in msort_with_tmp (n=7, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#7  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=14) at msort.c:53
#8  0x00007ffff77b5eb7 in msort_with_tmp (n=14, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#9  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=28) at msort.c:53
#10 0x00007ffff77b5eb7 in msort_with_tmp (n=28, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#11 msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=56) at msort.c:53
#12 0x00007ffff77b5eb7 in msort_with_tmp (n=56, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#13 msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=113) at msort.c:53
#14 0x00007ffff77b668f in msort_with_tmp (n=113, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#15 __GI___qsort_r (b=b@entry=0x249c0e0, n=n@entry=113, s=s@entry=8, cmp=0x7ffff7b9dbc0 <_compare_path_table>, arg=arg@entry=0x0) at msort.c:297
#16 0x00007ffff77b6758 in __GI_qsort (b=b@entry=0x249c0e0, n=n@entry=113, s=s@entry=8, cmp=<optimized out>) at msort.c:308
#17 0x00007ffff7b9f6cc in isoent_make_path_table_2 (a=a@entry=0x608010, depth=depth@entry=2, dir_number=dir_number@entry=0x7fffffffdd90, vdd=0x6085f8, vdd=0x6085f8) at libarchive/archive_write_set_format_iso9660.c:6931
#18 0x00007ffff7ba42e4 in isoent_make_path_table (a=0x608010) at libarchive/archive_write_set_format_iso9660.c:7037
#19 iso9660_close (a=0x608010) at libarchive/archive_write_set_format_iso9660.c:1932
#20 0x00007ffff7b91d52 in _archive_write_close (_a=0x608010) at libarchive/archive_write.c:513
#21 0x000000000040189e in create (filename=0x7fffffffe383 "test.iso", compress=0, argv=0x7fffffffe048) at appimagetool.c:193
#22 0x0000000000401588 in main (argc=4, argv=0x7fffffffe040) at appimagetool.c:80

# With rockridge ONLY


Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b9dbd4 in _compare_path_table (v1=0x249c108, v2=<optimized out>) at libarchive/archive_write_set_format_iso9660.c:6800
6800            cmp = p1->parent->dir_number - p2->parent->dir_number;
(gdb) bt
#0  0x00007ffff7b9dbd4 in _compare_path_table (v1=0x249c108, v2=<optimized out>) at libarchive/archive_write_set_format_iso9660.c:6800
#1  0x00007ffff77b6222 in msort_with_tmp (p=0x7fffffffdc90, b=0x249c108, n=2) at msort.c:83
#2  0x00007ffff77b5ecd in msort_with_tmp (n=2, b=0x249c108, p=0x7fffffffdc90) at msort.c:45
#3  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0f8, n=4) at msort.c:54
#4  0x00007ffff77b5ecd in msort_with_tmp (n=4, b=0x249c0f8, p=0x7fffffffdc90) at msort.c:45
#5  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=7) at msort.c:54
#6  0x00007ffff77b5eb7 in msort_with_tmp (n=7, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#7  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=14) at msort.c:53
#8  0x00007ffff77b5eb7 in msort_with_tmp (n=14, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#9  msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=28) at msort.c:53
#10 0x00007ffff77b5eb7 in msort_with_tmp (n=28, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#11 msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=56) at msort.c:53
#12 0x00007ffff77b5eb7 in msort_with_tmp (n=56, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#13 msort_with_tmp (p=0x7fffffffdc90, b=0x249c0e0, n=113) at msort.c:53
#14 0x00007ffff77b668f in msort_with_tmp (n=113, b=0x249c0e0, p=0x7fffffffdc90) at msort.c:45
#15 __GI___qsort_r (b=b@entry=0x249c0e0, n=n@entry=113, s=s@entry=8, cmp=0x7ffff7b9dbc0 <_compare_path_table>, arg=arg@entry=0x0) at msort.c:297
#16 0x00007ffff77b6758 in __GI_qsort (b=b@entry=0x249c0e0, n=n@entry=113, s=s@entry=8, cmp=<optimized out>) at msort.c:308
#17 0x00007ffff7b9f6cc in isoent_make_path_table_2 (a=a@entry=0x608010, depth=depth@entry=2, dir_number=dir_number@entry=0x7fffffffdd90, vdd=0x6085f8, vdd=0x6085f8) at libarchive/archive_write_set_format_iso9660.c:6931
#18 0x00007ffff7ba42e4 in isoent_make_path_table (a=0x608010) at libarchive/archive_write_set_format_iso9660.c:7037
#19 iso9660_close (a=0x608010) at libarchive/archive_write_set_format_iso9660.c:1932
#20 0x00007ffff7b91d52 in _archive_write_close (_a=0x608010) at libarchive/archive_write.c:513
#21 0x000000000040189e in create (filename=0x7fffffffe383 "test.iso", compress=0, argv=0x7fffffffe048) at appimagetool.c:193
#22 0x0000000000401588 in main (argc=4, argv=0x7fffffffe040) at appimagetool.c:80

@probonopd
Copy link
Author

probonopd commented Sep 12, 2016

So it looks like cmp = p1->parent->dir_number - p2->parent->dir_number; causes the issue?

in _compare_path_table (v1=0x249dfe8, v2=<optimized out>)
    at libarchive/archive_write_set_format_iso9660.c:6800
6800            cmp = p1->parent->dir_number - p2->parent->dir_number;

@probonopd
Copy link
Author

probonopd commented Sep 12, 2016

Changed the code like so:

    /* Compare parent directory number */
    fprintf(stderr,"p1->identifier: %s\n", p1->identifier);
    fprintf(stderr,"p2->identifier: %s\n", p2->identifier);
    fprintf(stderr,"p1->id_len: %i\n", p1->id_len);
    fprintf(stderr,"p2->id_len: %i\n", p2->id_len);
    fprintf(stderr,"p1->parent->dir_number: %i\n", p1->parent->dir_number);
    fprintf(stderr,"p2->parent->dir_number: %i\n\n", p2->parent->dir_number);
    cmp = p1->parent->dir_number - p2->parent->dir_number;
    if (cmp != 0)
        return (cmp);

With this result:

(...)

p1->identifier: TOOLS
p2->identifier: USR
p1->id_len: 5
p2->id_len: 3
p1->parent->dir_number: 2
p2->parent->dir_number: 2

p1->identifier: PRIVATE
p2->identifier: (null)
p1->id_len: 7
p2->id_len: 0
p1->parent->dir_number: 3

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b9dd4b in fprintf (__fmt=0x7ffff7bbe67b "p2->parent->dir_number: %i\n\n", 
    __stream=0x7ffff7b41540 <_IO_2_1_stderr_>)
    at /usr/include/x86_64-linux-gnu/bits/stdio2.h:97
97        return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,

p2->identifier: (null) looks suspicious to me.

Just speculating here: p2 has NULL identifier, hence is the root directory, hence has no parent, yet the cmp line tries to access its parent, which SEGFAULTs?

@kientzle
Copy link
Contributor

Just speculating here: p2 has NULL identifier, hence is the root directory, hence has no parent, yet the cmp line tries to access its parent, which SEGFAULTs?

I think that explains the crash but not the bug. I think the bug might be that the root directory should not be in this table at all. It's been a long time since I looked at this code and I need to look up what the path_table is for.

@kientzle
Copy link
Contributor

From ECMA-119 (also known as ISO9660), section 6.9:

For each directory in the directory hierarchy other than the Root Directory, the Path Table shall contain a record which identifies the directory, its Parent Directory and its location. The records in a Path Table shall be numbered starting from 1. The first record in the Path Table shall identify the Root Directory and its location.

So the compare function should always put the root directory before anything else. The following should achieve this:

diff --git a/libarchive/archive_write_set_format_iso9660.c b/libarchive/archive_write_set_format_iso9660.c
index c20e088..0aec5d1 100644
--- a/libarchive/archive_write_set_format_iso9660.c
+++ b/libarchive/archive_write_set_format_iso9660.c
@@ -6808,6 +6811,16 @@ _compare_path_table(const void *v1, const void *v2)
        p1 = *((const struct isoent **)(uintptr_t)v1);
        p2 = *((const struct isoent **)(uintptr_t)v2);

+       if (p1->parent == NULL || p1->identifier == NULL) {
+               if (p2->parent == NULL || p2->identifier == NULL) {
+                       return 0;
+               } else {
+                       return -1;
+               }
+       } else if (p2->parent == NULL || p2->identifier == NULL) {
+               return 1;
+       }
+
        /* Compare parent directory number */
        cmp = p1->parent->dir_number - p2->parent->dir_number;
        if (cmp != 0)

@kientzle
Copy link
Contributor

If it really needs the fix above, it seems like it should crash in other cases as well. There's something else going on here...

@probonopd
Copy link
Author

probonopd commented Sep 13, 2016

Does not crash on the cmp anymore, but now crashes in set_num_721(bp+7, np->parent->dir_number);:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ba21f3 in _write_path_table (vdd=0x6085f8, depth=2, type_m=0, 
    a=0x608010) at libarchive/archive_write_set_format_iso9660.c:4270
4270                set_num_721(bp+7, np->parent->dir_number);
(gdb) bt
#0  0x00007ffff7ba21f3 in _write_path_table (vdd=0x6085f8, depth=2, type_m=0, 
    a=0x608010) at libarchive/archive_write_set_format_iso9660.c:4270
#1  write_path_table (a=a@entry=0x608010, type_m=type_m@entry=0, 
    vdd=vdd@entry=0x6085f8)
    at libarchive/archive_write_set_format_iso9660.c:4301
#2  0x00007ffff7ba5908 in iso9660_close (a=0x608010)
    at libarchive/archive_write_set_format_iso9660.c:2048
#3  0x00007ffff7b92322 in _archive_write_close (_a=0x608010)
    at libarchive/archive_write.c:513
#4  0x000000000040189e in create (filename=0x7fffffffe35c "test.iso", 
    compress=0, argv=0x7fffffffdfe8) at appimagetool.c:193
#5  0x0000000000401588 in main (argc=4, argv=0x7fffffffdfe0)
    at appimagetool.c:80

and, for another set of test data (KDevelop-5.0.0-1-x86_64.AppImage), in set_num_722(bp+7, np->parent->dir_number);:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ba2112 in _write_path_table (vdd=0x6085f8, depth=2, type_m=1, 
    a=0x608010) at libarchive/archive_write_set_format_iso9660.c:4268
4268                set_num_722(bp+7, np->parent->dir_number);
(gdb) bt
#0  0x00007ffff7ba2112 in _write_path_table (vdd=0x6085f8, depth=2, type_m=1, 
    a=0x608010) at libarchive/archive_write_set_format_iso9660.c:4268
#1  write_path_table (a=a@entry=0x608010, type_m=type_m@entry=1, 
    vdd=vdd@entry=0x6085f8)
    at libarchive/archive_write_set_format_iso9660.c:4304
#2  0x00007ffff7ba5920 in iso9660_close (a=0x608010)
    at libarchive/archive_write_set_format_iso9660.c:2053
#3  0x00007ffff7b92322 in _archive_write_close (_a=0x608010)
    at libarchive/archive_write.c:513
#4  0x000000000040189e in create (filename=0x7fffffffe35c "test.iso", 
    compress=0, argv=0x7fffffffdfe8) at appimagetool.c:193
#5  0x0000000000401588 in main (argc=4, argv=0x7fffffffdfe0)
    at appimagetool.c:80

@probonopd probonopd changed the title Document ISO9660 better ISO9660 writing segfaults Sep 13, 2016
@kientzle
Copy link
Contributor

Can you reproduce this crash with a smaller number of files? If you could try running your sample program on just half of your input files, then keep cutting in half until you stop getting the crash, that would be interesting.

(It would be easier for me to reproduce your problem if I could do it with fewer input files.)

@probonopd
Copy link
Author

So you are saying you cannot reproduce the issue with an arbitrary large amount of files?

@kientzle kientzle added this to the 3.2.2 milestone Sep 19, 2016
@kientzle
Copy link
Contributor

I can consistently reproduce it with a particular directory of 598 files, but another with 1,000 files does not cause the problem.

It still happens without the zisofs option, so that's unrelated.

@kientzle
Copy link
Contributor

After some more experimenting, I've managed to narrow it further: the issue is triggered when you have a deep directory hierarchy. Here's my repro case:

$ mkdir -p t/1/2/3/4/5/6/7/8/9/10/11/12/13
$ echo hi > t/1/2/3/4/5/6/7/8/9/10/11/12/13/xxx
$ bsdtar cf t.iso --format=iso9660 t

That's a small enough case that I should be able to build a test around it and debug it.

@kientzle
Copy link
Contributor

@probonopd : Could you tell me the deepest directory in the tree you're trying to archive?

I think you can do this with something like the following:

$ cd <your root dir>; find . | sed -e 's|[^/]*|_|g' | sort | uniq

This just lists all the files, replaces the file/dir names with '_' and sorts the result. You should see something like this:

_
_/_
_/_/_
_/_/_/_
_/_/_/_/_
_/_/_/_/_/_
_/_/_/_/_/_/_
_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_/_

The last item in this example has 13 '_' characters; that's the deepest part of the tree.

Background: Plain ISO9660 has a limit of 7 directory levels. We use a standard "deep directory relocation" technique to handle directories deeper than that, but our current implementation only doubles the depth we can handle, resulting in a total limit of 13 directory levels.

We seem to have a bug that results in a messed-up directory tree in memory when we exceed that limit. That should never happen: We should either handle it or produce a legible error message.

I'll do some research to see if there are standard ways to push the deep-directory support further and also expand the tests to make sure we produce proper errors whenever we encounter directories deeper than we can handle.

@probonopd
Copy link
Author

probonopd commented Sep 24, 2016

$ find . | sed -e 's|[^/]*|_|g' | sort | uniq
_
_/_
_/_/_
_/_/_/_
_/_/_/_/_
_/_/_/_/_/_
_/_/_/_/_/_/_
_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_/_/_/_
_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_

That's 16 "_", and xorriso with RR seems to do this.

@kientzle
Copy link
Contributor

I would appreciate if you could experiment a bit and see if this explains what you're seeing: Do you consistently see the crash when the directory tree is more than 13 levels deep (and see it work correctly when the directory tree is shallower than that)?

From what I'm seeing, it does not matter the number of files, only the depth of directory nesting.

@probonopd
Copy link
Author

So far I can confirm that.

@kientzle kientzle modified the milestones: 3.3, 3.2.2 Oct 3, 2016
@kientzle kientzle changed the title ISO9660 writing segfaults ISO9660 writer segfaults on deep directories Oct 3, 2016
@kientzle
Copy link
Contributor

Commit 14144ae extends the existing iso9660 test to exercise directories that are 13 levels deep. If you modify the test to go a couple more levels, you get a failing test that can be used to demonstrate this bug.

@mmatuska
Copy link
Member

@probonopd any update on this?

@mmatuska mmatuska modified the milestones: 3.3, 3.4 May 16, 2019
@probonopd
Copy link
Author

probonopd commented May 16, 2019

Sorry, I have not followed up on this since I no longer have a need for writing ISO9660/zisofs.

@mmatuska mmatuska modified the milestones: 3.4, 3.5 May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants