Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
On 2022.06.26 08:25, Thomas Schmitt wrote: the Gentoo people did make sure that they placed a duplicate of the ESP image content on the ISO file system structure (Now, wouldn't it be *nice* if xorriso did that on its own... ;)) Wouldn't help if Gentoo hadn't thought of it, because they are still using mkisofs. Good point, I hadn't seen that they weren't using xorriso. On the other hand, and that's not to throw shade at xorriso because that's really a GRUB issue in the first place, that's probably why we're not running into the current pitfall of early UEFI boot breakage when extracting the ISO content to an NTFS partition, on account that grub-mkrescue, and by extension xorriso, was not being used to create this specific image... xorriso (1.5.5 in git) now has the proposed warning messages: xorriso : WARNING : EFI boot equipment is provided but no directory /EFI/BOOT xorriso : WARNING : will emerge in the ISO filesystem. A popular method to xorriso : WARNING : prepare a USB stick on MS-Windows relies on having in the xorriso : WARNING : ISO filesystem a copy of the EFI system partition tree. Nice, but, as I pointed out, that "popular" method is not just for Windows and, in the context of trying to enlighten Linux distro maintainers about the pleas of their users, I see it as a bit reductive to present the issue as mostly a Windows centric problem, when it isn't. As I pointed out on the GRUB mailing list, there can be major advantages to using the EFI file extraction method, even if you're using an OS where dd is easily accessible and is promoted as the default method of dealing with an ISOHybrid. My offer stands to add another warning line with the URL of a document which describes the needs of the unpacking method(s). I appreciate that, and I'll get back to you on it off list. Haven't had a chance to do that yet due to prioritization (which is also the reason I needed to push a BETA of Rufus 3.19 on Friday, and not because there was a particularly important bug to fix). I would also be willing to adopt such a description for the docs of libisofs and GNU xorriso, if your license permits it. As you probably guessed, I'm still seeing duplication of the ESP image content as something that xorriso should be doing, and just to give you an idea of what I'm now leaning towards when I get back to you off list (still no idea when I'll do that, as this xorriso matter is not that high on my current priority list right now) would be to enforce a 2048-byte cluster size for the FAT image, since people are explicitly creating that image for xorriso and therefore have easy control over its parameters and last time I checked one can use a 2048 cluster size for FAT volumes up to at least 8GB in size. Then, instead of trying to extract and duplicate the files, xorriso should "simply" be able to map the existing FAT files into the ISO-9660 index, as (provided that there is no fragmentation of the FAT image, which would be another item that xorriso would need to checked) everything should be nicely laid out for it to do so. Of course, I haven't looked at it in details, so maybe there's a major roadblock there. But if achievable, and despite the extra constraint it would push on the ESP image creation (with xorriso returning an error if the ESP image fed to it doesn't use a 2048-byte cluster size), I think this could be an elegant solution to the issue. their build of GRUB 2.0 does include the ntfs module [...] the boot kernel is still missing an NTFS driver, These and any other constraints should be documented in a quick-to-read manner. Yup, I agree, and I have some vague plan to do just that when I can free up some time to deal with this specific matter. Regards, /Pete
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi, Pete Batard wrote: > Not too sure why you jumped on taking the blame for introducing the issue, > as I'm pretty sure I'm the one who did You are right. The problem is already in the branch "pbatard-multiextent2". So i only adopted it when i started my git branch "pragmatic-multiextent" from that branch. From then on i was a good foster parent until you developed the changeset which finally got into the main branch: https://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=d758fa2253cca062e92ad0754a64c15a854db4ff https://lists.gnu.org/archive/html/libcdio-devel/2020-05/msg00017.html (Reason for my involvement was national penance after FIFA World Cup 2018.) > the Gentoo > people did make sure that they placed a duplicate of the ESP image content > on the ISO file system structure (Now, wouldn't it be *nice* if xorriso did > that on its own... ;)) Wouldn't help if Gentoo hadn't thought of it, because they are still using mkisofs. xorriso (1.5.5 in git) now has the proposed warning messages: xorriso : WARNING : EFI boot equipment is provided but no directory /EFI/BOOT xorriso : WARNING : will emerge in the ISO filesystem. A popular method to xorriso : WARNING : prepare a USB stick on MS-Windows relies on having in the xorriso : WARNING : ISO filesystem a copy of the EFI system partition tree. My offer stands to add another warning line with the URL of a document which describes the needs of the unpacking method(s). I would also be willing to adopt such a description for the docs of libisofs and GNU xorriso, if your license permits it. > their build of GRUB 2.0 does include the ntfs module [...] > the boot kernel is still missing an NTFS driver, These and any other constraints should be documented in a quick-to-read manner. (Consider to write a very concise checklist section and to give detailed motivation in further parts of the document. As i know my distro users they have about 5 minutes time per month for this aspect of their work.) Have a nice day :) Thomas
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi Thomas,
I can confirm that your patch solves both the issues I was seeing in
Rufus. Not too sure why you jumped on taking the blame for introducing
the issue, as I'm pretty sure I'm the one who did when I thought that
we'd always had valid ISO9660 names when dealing with multiextents, and
can point to the comments going that way in the first proposal at [1]
(from which I believe the subsequent proposals were derived).
Also, I haven't analysed the isolinux error report to a great extent,
but it looks to me that, because of the name screwup, we're ending up
trying to process a multiextent part that does not belong to the
isolinux/ directory while we're traversing that directory, hence the report.
At least, what a quick debug session with the problematic code showed me
in Rufus is that the "Bad directory information for isolinux" is a
direct consequence of getting a "Non consecutive multiextent file parts
for ''" when calling _iso9660_dir_to_statbuf() while we're traversing
isolinux/ and getting a NULL p_stat as a result. So, indeed, if you fix
the consecutive multiextent error, then you fix the isolinux error, even
though at first glance those two seemed unrelated.
At any rate, your patch looks good to me, so thanks a lot for
investigating and producing a fix so quickly!
I have validated that, once applied to Rufus, it can create media from
the Gentoo image without issues, so I'll integrate the fix for the 3.19
release... though, and this is mostly addressed at Ben, this still
doesn't result in a media that boots Gentoo properly because, even
though the Gentoo people did make sure that they placed a duplicate of
the ESP image content on the ISO file system structure (Now, wouldn't it
be *nice* if xorriso did that on its own... ;)) and that their build of
GRUB 2.0 does include the ntfs module (Props to the Gentoo maintainers
for thinking about doing that), it looks like the boot kernel is still
missing an NTFS driver, which ultimately prevents the installer from
being able to mount the media and proceed...
That's actually the curse of having to contend with files that are
larger than 4 GB in your installation image: Besides running into issues
with poorly written multiextent support (which I'll happily take the
blame for), you also have to pretty much add support for NTFS every step
of the way unless you're content with restricting your users to
duplicating your media with dd only, since NTFS is becoming the de-facto
replacement file system for when FAT32 is no longer suitable...
Regards,
/Pete
[1]
https://lists.gnu.org/archive/html/libcdio-devel/2018-06/msg1.html,
which
On 2022.06.25 11:05, Thomas Schmitt wrote:
Hi,
i pushed a new branch "joliet_multi_extent" with a single commit
Bug fix: Recognition of multi-extent files was broken if Joliet is present
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=joliet_multi_extent=4c840665c6d9cf2ff1cf0cd12f91b25030776c74
A new function _iso9660_recname_to_cstring() implements the conversion
which _iso9660_dir_to_statbuf() now needs at three occasions.
It can allocate the memory for the result string or put it into a
submitted memory area. (I doubt that both modes together make sense.)
A cosmetic change was to equip the recognition of "." and ".." with
curly brackets so that i could replace
else if (u_joliet_level) {
by
} else {
-
I began to explore the
"Bad directory information for isolinux"
problem, which Pete Batard reported.
The commited code lets iso-info report properly and without warning the
content of Gentoo's livegui-amd64-20220605T170549Z.iso
I then took my provisory fix of yesterday and blocked the recognition
of u_joliet_level in both new occasions.
This yielded an endless stream of "." and ".." filenames. Not what Pete
reported.
But if i only disable one of the two new Joliet name recognitions, i get
the "Bad directory information for isolinux" message and a similar one
about directory "/snapshots". It does not matter which of the two i keep
disabled.
The same happens if i checkout branch "master" and compile it.
So i looks like i fixed this too, but without fully understanding what
was wrong initially.
Have a nice day :)
Thomas
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi,
i pushed a new branch "joliet_multi_extent" with a single commit
Bug fix: Recognition of multi-extent files was broken if Joliet is present
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=joliet_multi_extent=4c840665c6d9cf2ff1cf0cd12f91b25030776c74
A new function _iso9660_recname_to_cstring() implements the conversion
which _iso9660_dir_to_statbuf() now needs at three occasions.
It can allocate the memory for the result string or put it into a
submitted memory area. (I doubt that both modes together make sense.)
A cosmetic change was to equip the recognition of "." and ".." with
curly brackets so that i could replace
else if (u_joliet_level) {
by
} else {
-
I began to explore the
"Bad directory information for isolinux"
problem, which Pete Batard reported.
The commited code lets iso-info report properly and without warning the
content of Gentoo's livegui-amd64-20220605T170549Z.iso
I then took my provisory fix of yesterday and blocked the recognition
of u_joliet_level in both new occasions.
This yielded an endless stream of "." and ".." filenames. Not what Pete
reported.
But if i only disable one of the two new Joliet name recognitions, i get
the "Bad directory information for isolinux" message and a similar one
about directory "/snapshots". It does not matter which of the two i keep
disabled.
The same happens if i checkout branch "master" and compile it.
So i looks like i fixed this too, but without fully understanding what
was wrong initially.
Have a nice day :)
Thomas
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi, Rocky Bernstein wrote: > My suggestion is to get your patches in now. The posted patch is still ugly and only not fatally wrong because of the use of calloc(). Look at the strncpy() calls. Not even the indentation is correct, because i ripped the code pieces from a few lines down in the same function and only roughly adapted them. And then there is a comment which is now wrong. Tomorrow i'll try to follow the usual procedure, which i have to re-learn each time i make my yearly contribution to libcdio. You'll get a branch with a nice commit ... i hope. Pete Batard wrote: > I am in the process of releasing Rufus 3.19 BETA Ah. The joy of a last minute bug fix. (Only good that nobody looks up who of us both is to blame.) > Note that I am seeing another issue with the Gentoo image with a report of > "Bad directory information for isolinux" (from [1]), which I think might > have to do with the same issue, as it goes away with Joliet disabled and > this message is produced when the _iso9660_dir_to_statbuf(..., > p_iso->u_joliet_level) call fails. But that can hardly have to do with a multi-extent file. Well, Joliet and ISO 9660 / ECMA-119 are completely separate directory trees. The problem does not necessarily have to be confined in the only HAVE_JOLIET code piece in _iso9660_dir_to_statbuf(). Now downloading http://distfiles.gentoo.org/releases/amd64/autobuilds/20220605T170549Z/livegui-amd64-20220605T170549Z.iso so i can have an own peek ... tomorrow. wget says "551KB/s eta 2h 39m". (Drop a note if you find the problem in the meantime.) Have a nice day :) Thomas
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Comments in line.
On Fri, Jun 24, 2022 at 8:40 AM Thomas Schmitt wrote:
> Hi,
>
> Ben Kohler wrote:
> > I've found that for an iso
> > created with (cdrtools) mkisofs -J -iso-level 3, libcdio tools like
> > iso-info and iso-read are not able to handle multi-extent files.
>
> It's a pair of bugs in function _iso9660_dir_to_statbuf() of libcdio
> source file lib/iso9660/iso9660_fs.c .
>
> ---
> Symptoms:
>
> I can reproduce this with a xorriso-made ISO too, which has Rock Ridge
> additionally to Joliet.
> (mkisofs with -R doesn't help either. genisoimage is safe because it
> does not obey -iso-level 3 and aborts already on image production.)
>
> A workaround for iso-info seems to be option
> --no-joliet
> man iso-read does not mention this option.
>
> A run of iso-info without --no-joliet complains:
>
> ++ WARN: Non consecutive multiextent file parts for ''
>
> ---
> Diagnosis:
>
> The message comes from _iso9660_dir_to_statbuf() in iso9660_fs.c.
> It is supposed to report a filename in the quotation marks.
> In order to get there, a re-used p_stat has to be submitted by the
> caller.
>
> The first bug is this:
>
> When iso9660_ifs_readdir() calls _iso9660_dir_to_statbuf() with the first
> directory entry of the multi-extent file it gets back an empty string as
> file name, because the conversion from UCS-2 to UTF-8 is missing if the
> ISO_MULTIEXTENT bit is set in the directory record.
> This is probably my fault due to the assumption that libcdio already
> deals with C strings at that point of processing.
> See
>
> https://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=d758fa2253cca062e92ad0754a64c15a854db4ff
> "Use the plain ISO-9660 name when dealing with a multiextent file part"
>
> The second bug is similar:
>
> The test which leads to the warning message compares the C string which
> holds
> the previously read file name with the UCS-2 array which holds the name of
> the follow-up directory record.
> So even if the UTF-8 conversion missing in above bug is done, this test
> still
> detects that the string length does not match the array byte size and thus
> throws the warning and prevents the proper merging of both directory
> records.
> See
>
> https://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=d758fa2253cca062e92ad0754a64c15a854db4ff
> "Only resolve the full filename when we're not dealing with extent"
>
> ---
> Code to test:
>
> I now have an ugly contraption by which iso-info recognizes a multi-extent
> file even if Joliet is present. The base is an outdated repo clone.
> I will have to get a fresh git state and try to find my cheat sheet
> which describes how to submit changes to libcdio.
>
> The current diff (minus all my debugging printfs) is:
>
> diff --git a/lib/iso9660/iso9660_fs.c b/lib/iso9660/iso9660_fs.c
> index be693c7..a84b9fc 100644
> --- a/lib/iso9660/iso9660_fs.c
> +++ b/lib/iso9660/iso9660_fs.c
> @@ -866,8 +866,25 @@ _iso9660_dir_to_statbuf (iso9660_dir_t *p_iso9660_dir,
>if ((p_iso9660_dir->file_flags & ISO_MULTIEXTENT) == 0) {
> /* Check if this is the last part of a multiextent file */
> if (!first_extent) {
> - if (strlen(p_stat->filename) != i_fname ||
> - strncmp(p_stat->filename, _iso9660_dir->filename.str[1],
> i_fname) != 0) {
> + cdio_utf8_t *p_psz_out = NULL;
> + int bad_multi;
> +
> +#ifdef HAVE_JOLIET
> + if (u_joliet_level) {
> + int i_inlen = i_fname;
> + if (!cdio_charset_to_utf8(_iso9660_dir->filename.str[1], i_inlen,
> + _psz_out, "UCS-2BE")) {
> + goto fail;
> +}
> + } else
> +#endif /*HAVE_JOLIET*/
> + {
> +p_psz_out = calloc(1, i_fname + 1);
> + strncpy (p_psz_out, _iso9660_dir->filename.str[1], i_fname);
> + }
> + bad_multi = (strcmp(p_stat->filename, p_psz_out) != 0);
> + free(p_psz_out);
> + if (bad_multi) {
> cdio_warn("Non consecutive multiextent file parts for '%s'",
> p_stat->filename);
> goto fail;
> @@ -916,7 +933,22 @@ _iso9660_dir_to_statbuf (iso9660_dir_t *p_iso9660_dir,
> }
>} else {
>/* Use the plain ISO-9660 name when dealing with a multiextent file
> part */
> - strncpy(p_stat->filename, _iso9660_dir->filename.str[1], i_fname);
> +#ifdef HAVE_JOLIET
> + if (u_joliet_level) {
> + int i_inlen = i_fname;
> + cdio_utf8_t *p_psz_out = NULL;
> + if (cdio_charset_to_utf8(_iso9660_dir->filename.str[1], i_inlen,
> + _psz_out, "UCS-2BE")) {
> + strncpy(p_stat->filename, p_psz_out, i_fname);
> + free(p_psz_out);
> +} else {
> + goto fail;
> +}
> + } else
> +#endif /*HAVE_JOLIET*/
> + {
> + strncpy (p_stat->filename,
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi Ben, Hi Thomas, On 2022.06.24 15:53, Ben Kohler wrote: On Fri, Jun 24, 2022 at 7:39 AM Thomas Schmitt wrote: My ACTUAL use-case/goal here is to get Rufus to successfully extract & copy a Gentoo LiveGUI ISO [1] which contains a 5GB squashfs image inside. But once I found that Rufus was using libcdio inside, I began testing with libcdio CLI tools. I believe that this patch would allow Rufus to succeed with the Gentoo ISO. For the record, you can already disable Joliet support with - in Rufus, which I tested, allows it to proceed with the livegui-amd64-20220605T170549Z.iso image. I haven't tested actual boot with the resulting media at this stage. Note that I am seeing another issue with the Gentoo image with a report of "Bad directory information for isolinux" (from [1]), which I think might have to do with the same issue, as it goes away with Joliet disabled and this message is produced when the _iso9660_dir_to_statbuf(..., p_iso->u_joliet_level) call fails. I am in the process of releasing Rufus 3.19 BETA (which must happen today for varied reasons), but I'll see if I can squeeze Thomas' patch for the 3.19 release, which should be about one week for now. I can't promise anything though, especially as I'd want to eliminate the isolinux issue for that release too... Regards, /Pete [1] http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/iso9660/iso9660_fs.c#n1238
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
On Fri, Jun 24, 2022 at 7:39 AM Thomas Schmitt wrote: > > -- > > @ Ben Kohler: > You'd do me a big favor if you could test this with your use cases. > Hi Thomas, Thanks for the patch. I can't quite get the first hunk to apply cleanly with patch but I've hand-applied all the changes. This does indeed allow iso-info to list the file correctly without warnings/errors, and iso-read is able to extract the file. My ACTUAL use-case/goal here is to get Rufus to successfully extract & copy a Gentoo LiveGUI ISO [1] which contains a 5GB squashfs image inside. But once I found that Rufus was using libcdio inside, I began testing with libcdio CLI tools. I believe that this patch would allow Rufus to succeed with the Gentoo ISO. Thanks, Ben [1] http://distfiles.gentoo.org/releases/amd64/autobuilds/20220605T170549Z/livegui-amd64-20220605T170549Z.iso
Re: [Libcdio-devel] libcdio tools cannot read multi-exent files with Joliet exensions on
Hi,
Ben Kohler wrote:
> I've found that for an iso
> created with (cdrtools) mkisofs -J -iso-level 3, libcdio tools like
> iso-info and iso-read are not able to handle multi-extent files.
It's a pair of bugs in function _iso9660_dir_to_statbuf() of libcdio
source file lib/iso9660/iso9660_fs.c .
---
Symptoms:
I can reproduce this with a xorriso-made ISO too, which has Rock Ridge
additionally to Joliet.
(mkisofs with -R doesn't help either. genisoimage is safe because it
does not obey -iso-level 3 and aborts already on image production.)
A workaround for iso-info seems to be option
--no-joliet
man iso-read does not mention this option.
A run of iso-info without --no-joliet complains:
++ WARN: Non consecutive multiextent file parts for ''
---
Diagnosis:
The message comes from _iso9660_dir_to_statbuf() in iso9660_fs.c.
It is supposed to report a filename in the quotation marks.
In order to get there, a re-used p_stat has to be submitted by the
caller.
The first bug is this:
When iso9660_ifs_readdir() calls _iso9660_dir_to_statbuf() with the first
directory entry of the multi-extent file it gets back an empty string as
file name, because the conversion from UCS-2 to UTF-8 is missing if the
ISO_MULTIEXTENT bit is set in the directory record.
This is probably my fault due to the assumption that libcdio already
deals with C strings at that point of processing.
See
https://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=d758fa2253cca062e92ad0754a64c15a854db4ff
"Use the plain ISO-9660 name when dealing with a multiextent file part"
The second bug is similar:
The test which leads to the warning message compares the C string which holds
the previously read file name with the UCS-2 array which holds the name of
the follow-up directory record.
So even if the UTF-8 conversion missing in above bug is done, this test still
detects that the string length does not match the array byte size and thus
throws the warning and prevents the proper merging of both directory
records.
See
https://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=d758fa2253cca062e92ad0754a64c15a854db4ff
"Only resolve the full filename when we're not dealing with extent"
---
Code to test:
I now have an ugly contraption by which iso-info recognizes a multi-extent
file even if Joliet is present. The base is an outdated repo clone.
I will have to get a fresh git state and try to find my cheat sheet
which describes how to submit changes to libcdio.
The current diff (minus all my debugging printfs) is:
diff --git a/lib/iso9660/iso9660_fs.c b/lib/iso9660/iso9660_fs.c
index be693c7..a84b9fc 100644
--- a/lib/iso9660/iso9660_fs.c
+++ b/lib/iso9660/iso9660_fs.c
@@ -866,8 +866,25 @@ _iso9660_dir_to_statbuf (iso9660_dir_t *p_iso9660_dir,
if ((p_iso9660_dir->file_flags & ISO_MULTIEXTENT) == 0) {
/* Check if this is the last part of a multiextent file */
if (!first_extent) {
- if (strlen(p_stat->filename) != i_fname ||
- strncmp(p_stat->filename, _iso9660_dir->filename.str[1], i_fname)
!= 0) {
+ cdio_utf8_t *p_psz_out = NULL;
+ int bad_multi;
+
+#ifdef HAVE_JOLIET
+ if (u_joliet_level) {
+ int i_inlen = i_fname;
+ if (!cdio_charset_to_utf8(_iso9660_dir->filename.str[1], i_inlen,
+ _psz_out, "UCS-2BE")) {
+ goto fail;
+}
+ } else
+#endif /*HAVE_JOLIET*/
+ {
+p_psz_out = calloc(1, i_fname + 1);
+ strncpy (p_psz_out, _iso9660_dir->filename.str[1], i_fname);
+ }
+ bad_multi = (strcmp(p_stat->filename, p_psz_out) != 0);
+ free(p_psz_out);
+ if (bad_multi) {
cdio_warn("Non consecutive multiextent file parts for '%s'",
p_stat->filename);
goto fail;
@@ -916,7 +933,22 @@ _iso9660_dir_to_statbuf (iso9660_dir_t *p_iso9660_dir,
}
} else {
/* Use the plain ISO-9660 name when dealing with a multiextent file part
*/
- strncpy(p_stat->filename, _iso9660_dir->filename.str[1], i_fname);
+#ifdef HAVE_JOLIET
+ if (u_joliet_level) {
+ int i_inlen = i_fname;
+ cdio_utf8_t *p_psz_out = NULL;
+ if (cdio_charset_to_utf8(_iso9660_dir->filename.str[1], i_inlen,
+ _psz_out, "UCS-2BE")) {
+ strncpy(p_stat->filename, p_psz_out, i_fname);
+ free(p_psz_out);
+} else {
+ goto fail;
+}
+ } else
+#endif /*HAVE_JOLIET*/
+ {
+ strncpy (p_stat->filename, _iso9660_dir->filename.str[1], i_fname);
+ }
}
iso9660_get_dtime(&(p_iso9660_dir->recording_time), true, &(p_stat->tm));
--
@ Ben Kohler:
You'd do me a big favor if you could test this with your use cases.
@ All:
I am unhappy with the
