[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libcdio-devel] Rock Ridge and libisofs/xorriso 'AL' extension

From: Thomas Schmitt
Subject: Re: [Libcdio-devel] Rock Ridge and libisofs/xorriso 'AL' extension
Date: Sun, 30 Jul 2017 09:55:29 +0200


here are my preliminary sunday morning comments. I will send a new patch
(planned for today) which addresses them.


The problematic code gesture exists several times in iso9660_fs.c.
You now changed an occurence in _fs_stat_traverse() which i did not
encounter while investigating.

The one which i encountered during stepping with gdb is in
function iso9660_fs_readdir():

The code gestures look very similar. This calls for a common function
which checks for zero directory length.
It may also do the check for block crossing which i propose below.

Still existing in the git branch are the "offset++" gestures at:

I'd say they all need the same safety cap.


You did not interpret my experimental #ifdefs correctly:


> +     offset += ISO_BLOCKSIZE - (offset % ISO_BLOCKSIZE);
>       offset++;

The line "offset++" was to be replaced by the "offset+=" line.
(It was in the #else-case of the #ifdef in my test proposal.)

As it is now, it would skip the first byte in the next block
and thus misread any directory which consists of more than one block.


-       offset++

But as said, this should become a call to a new function, anyways.


> + libisofs of the libburnia project uses a directory
> + record length of 0 as an indicator to advance to the next block

The Linux kernel does it too.
Probably that's the stronger reference to quote.

> > 1:
> > libcdio did not notice that the mistaken directory record (caused by the
> > stray byte with value 128) reached over the end of the current block.
> > ...
> > 2:
> > libcdio seems to assume a valid chain of SUSP fields when it encounters
> > additional space after the end of ISO 9660 file name and possible padding

> My understanding is that these two issues still would need resolution.

Yes. My proposal was meant only to fix the memory corruption caused
by the bad ISO of bug 45015 without discriminating good ISOs if they
contain non-Rock-Ridge SUSO fields.

Fixing problem 2 is not without the risk to exclude half-good ISOs
which for some reason have Rock Ridge without announcing it properly.
Linux is similarly tolerant than libcdio.

So possibly one will only address problem 1 to raise the propbability
to catch fully bad ISOs before they can cause havoc in the code.

I will include this in my proposal of a common function to check all
five occasions of the handling of directory block ends.

I'll begin now with implementation and testing.
This time without deceiving macro alternatives (they help while one
is unsure whether one does the right thing).

What is the preferred form of an applicable patch ?
Mail inline ? Attachment ?

Must i clone the the git branch TS-RockRidge-Fix first ?
(git still gives me creeps. It is so optimistic about merging.)

Have a nice day :)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]