Dear Libcdio developers,
I need your help with the recent ABI changes in libiso9660.
The 2.2.0 release contains the ABI bump for libiso9660 due to the
addition of new fields/members in a few structs. I understand that
previous versions (also recently released) did not have the bump which
was rectified with version 2.2.0. All is well with regards to this.
However, while working on the Debian update for the sub-libraries in
the parent libcdio project, I got a few Segmentation Faults in the
32-bits architectures. I tried to dig as deep as I could, and even
though I have not yet precisely traced the events that cause the issue,
I have a guess.
I have only a couple of question (lines prefixed with '*'), even though
the email is huge (sorry about that - I'm bad at writing).
Let me first explain what is the condition and you can maybe stop me
right there in case you think I'm doing something wrong.
Mixing old and new sub-libraries
--------------------------------
The libcdio project produces several sub-libraries, including
libcdio.so and libiso9660.so, in version 2.1.0, these would be their
filanames (and SONAMEs) in the filesystem:
libcdio.so.19.0.0
libiso9660.so.11.0.0
Whereas in version 2.2.0, they would be:
libcdio.so.19.0.0
libiso9660.so.12.0.0
In Debian (and probably likewise in other distros), it is possible to
install both versions of libiso9960 concurrently. This is useful when
you have applications that you don't want to recompile, so you keep
distributing the old library for users who have applications that
linked against libiso9660.so.11.
However, only a single version of libcdio can be installed.
In Debian, one could install libiso9660.so.11 (from libcdio 2.1.0),
libiso9660.so.12 (from libcdio 2.2.0), and libcdio.so.19 (from libcdio
2.2.0).
And an application that linked against libiso9660.so.11 (and
libcdio.so.19) will use sub-libraries from different versions of the
libcdio project. And this is what causes the Segmentation Fault.
* Should Debian stop right there and never allow this mismatching?
What does the error look like
-----------------------------
In GDB, it looks like this:
Program received signal SIGSEGV, Segmentation fault.
0xf7ec983c in ?? () from /lib/i386-linux-gnu/libc.so.6
(gdb) bt
#0 0xf7ec983c in ?? () from /lib/i386-linux-gnu/libc.so.6
#1 0xf7f87c43 in iso9660_fs_read_pvd (p_cdio=0x5655a1c0, p_pvd=0xf7f96c22
<cdio_generic_free+3>) at iso9660_fs.c:635
#2 0xf7f87cce in iso9660_fs_read_superblock (p_cdio=0x5655a1c0,
iso_extension_mask=255 '\377') at iso9660_fs.c:659
#3 0xf7f8858e in _fs_stat_root (p_cdio=0x5655a1c0) at iso9660_fs.c:958
#4 0xf7f88fc1 in iso9660_fs_stat (p_cdio=0x5655a1c0, psz_path=0xf7f8c9f3 "/")
at iso9660_fs.c:1227
#5 0xf7f891fa in iso9660_fs_readdir (p_cdio=0x5655a1c0, psz_path=0xf7f8c9f3
"/") at iso9660_fs.c:1364
#6 0xf7f896ea in find_lsn_recurse (p_image=0x5655a1c0,
iso9660_readdir=0xf7f891ae <iso9660_fs_readdir>, psz_path=0xf7f8c9f3 "/",
lsn=26, ppsz_full_filename=0xffffd508) at iso9660_fs.c:1520
#7 0xf7f89a61 in iso9660_find_fs_lsn (p_cdio=0x5655a1c0, i_lsn=26) at
iso9660_fs.c:1603
#8 0x56556211 in main () at rep.c:18
(the reproducer is further down this message)
Changes to the opaque CdIo_t (aka struct _CdIo)
-----------------------------------------------
The CdIo_t type, which is returned by functions like cdio_open, is an
opaque type, as mentioned in the public header (cdio/cdio.h):
/** This is an opaque structure for the CD object. */
typedef struct _CdIo CdIo_t;
However, libiso9960 knows about its internal details, and access
fields/member directly, such as in iso9660_fs_read_superblock:
bool
iso9660_fs_read_superblock (CdIo_t *p_cdio,
iso_extension_mask_t iso_extension_mask)
{
if (!p_cdio) return false;
generic_img_private_t *p_env = (generic_img_private_t *) p_cdio->env;
~~~~~~~~~~~
Direct access here ^
However, struct _CdIo changed between versions 2.1.0 and 2.2.0. More
specifically, it change with commit ID e35135114e71 and now has the
additional member/field `cdio_header_t header'. This changes the size
of `struct _CdIo' and likely the offsets of the fields.
My guess is that this is what's causing the segfaults.
While I understand that CdIo_t is a private type, it is only private
with regards to the libcdio project. One could say that Libiso9960 made
it public by accessing fields directly. If so, libcdio needs a SONAME
bump. Conversely, one would say that libiso9660 is currently making
invalid access and should be fixed and should use accessor methods
instead.
* Do you have an opinion on this?
Reproducer
----------
I'm cheating here, because I'm reusing the CUE/BIN pair from the test
suite in libdevice-cdio-perl. Anyhow, to remove this Perl package from
the equation, the following program could be used:
#include <err.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <cdio/cdio.h>
#include <cdio/iso9660.h>
int
main(void)
{
CdIo_t *cdio;
iso9660_stat_t *stat;
cdio = cdio_open("isofs-m1.cue", DRIVER_UNKNOWN);
if (cdio == NULL)
err(EXIT_FAILURE, "unable to open disk file");
stat = iso9660_fs_find_lsn(cdio, 26);
if (stat == NULL)
err(EXIT_FAILURE, "unable to find LSN");
cdio_destroy(cdio);
return 0;
}
The data files (isofs-m1.cue and isofs-m1.bin) can be found in
libdevice-cdio-perl, e.g.:
https://sources.debian.org/src/libdevice-cdio-perl/2.0.0-2/data/
You must build this against libis9660.so.11 (from libcdio 2.1.0), but
update libcdio.so.19 (to libcdio 2.2.0)
The segmentation fault only happens in 32-bits architectures, and I
still can't explain why.
Debian specifics
----------------
In the interest of being totally upfront, I should also mention that
Debian carries a patch for the lseek/fseeko issues in 32-bits
architectures.
Since these functions deal file access, I simply assumed that this
patch should not cause this Segmentation Fault, anyhow, the patch is as
follows:
--- libcdio.orig/lib/driver/_cdio_stdio.h
+++ libcdio/lib/driver/_cdio_stdio.h
@@ -22,6 +22,12 @@
#include "_cdio_stream.h"
+#include <features.h>
+#if defined(_FILE_OFFSET_BITS) && defined(__REDIRECT) && (_FILE_OFFSET_BITS ==
64)
+#define lseek64 lseek
+#define fseeko64 fseeko
+#endif
+
/*!
Initialize a new stdio stream reading from pathname.
A pointer to the stream is returned or NULL if there was an error.
(reference:
https://sources.debian.org/src/libcdio/2.2.0-2/debian/patches/arm-t64-redirect-fix.patch/)
Cheers,
Gabriel
PS: The Debian issue is being tracked here: https://bugs.debian.org/1094736
