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

[Rocky Bernstein]

Sorry the hand patching didn't work out, and that's why a more direct means
is preferable. Here are the commands to run. Start off in a directory that
doesn't have a libcdio file or folder in it. Then run:

  git clone --single-branch --branch TS-RockRidge-Fix ssh://
address@hidden/srv/git/libcdio.git
  cd libcdio

After that make your changes. At each logical transaction of useful change
issue:

    git commit .

Make sure to do this is the top-level libcdio folder that you cd'ed into.
This will bring up an editor where you enter a commit message describing
what the collection of changes was about. So make sure the EDITOR
environment variable is set to something reasonable for you.

At place where you want others to look over the code issue

   git push

That's it. Thanks


I still am not sure I understand the comment:

+              The formula does not exactly round up, as it enlarges offset
+              even if it encounters (offset % ISO_BLOCKSIZE) == 0 .
+              Then the block would be completely 0. Unplausible.



On Sun, Jul 30, 2017 at 3:55 AM, Thomas Schmitt <address@hidden> wrote:

> Hi,
>
> 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():
>   http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/
> iso9660/iso9660_fs.c?h=TS-RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c1
> 8e7fa2df13#n1316
>
> 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:
>
>   http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/
> iso9660/iso9660_fs.c?h=TS-RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c1
> 8e7fa2df13#n1103
>   http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/
> iso9660/iso9660_fs.c?h=TS-RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c1
> 8e7fa2df13#n1318
>   http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/
> iso9660/iso9660_fs.c?h=TS-RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c1
> 8e7fa2df13#n1401
>   http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/
> iso9660/iso9660_fs.c?h=TS-RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c1
> 8e7fa2df13#n1613
>
> I'd say they all need the same safety cap.
>
> ------------------------------------------------------------------------
>
> You did not interpret my experimental #ifdefs correctly:
>
> > http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=TS-
> RockRidge-Fix&id=7db119850977f5fa37d6d5e8ed14c18e7fa2df13
>
> > +     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.
>
> So:
>
> -       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 :)
>
> Thomas
>
>
>