Hi, following will be 6 patch proposals for isohybrid.c 1: Encode GPT partition names as UTF-16LE 2: Correct blocking factor in APM partition block counts 3: Correct end block address of first GPT partition 4: Write GPT backup to the very end of the image 5: Change all fseek(3) to fseeko(3) 6: Introduce option --mbr and make isohybrid.c compilable standalone If the form needs adjustments, i whould be able to reproduce the patches. The state after patch 6 was tested by re-hybdridization of yetserday's Fedora-Live-Desktop-x86_64-20-1.iso. The result was loaded by xorriso which then assessed and reported the MBR, GPT, and APM equipment. One false positive complaint appears: GPT backup problems: Implausible header LBA 1951743, Implausible array LBA 1951711 It could be avoided by adjusting the ISO filesystem size to the size of the image file. Regrettably this causes isohybrid.c to add another MiB of padding if the image is isohybridized again. So i will ponder how to recognize the habit of isohybrid.c as a tolerable deviation - or how to let isohybrid.c recognize that the filesystem already has a backup GPT at its end, so that no new space is needed. Have a nice day :) Thomas
Thomas Schmitt
2014-Jun-22 19:11 UTC
[syslinux] [PATCH] utils/isohybrid.c: 001 Encode GPT partition names as UTF-16LE
The worst sin of isohybrid.c was to compose GPT partition names by
8-bit characters and to memcpy() them as if they were 16 bit wide.
GPT names are encoded as UTF-16LE. It is trivial to create this encoding
from 7-bit ASCII.
This change introduces two byte arrays with the desired UTF-16LE names
which replace the string constants "ISOHybrid ISO" and
"ISOHybrid".
--- isohybrid.c.orig 2014-06-22 20:07:19.000000000 +0200
+++ isohybrid.c.001_gpt_name 2014-06-22 20:04:30.000000000 +0200
@@ -749,6 +749,11 @@ initialise_gpt(uint8_t *gpt, uint32_t cu
struct gpt_part_header *part;
int hole = 0;
int gptsize = 128 / 4 + 2;
+ char part_name_iso[] = {'I', 0, 'S', 0, 'O', 0,
'H', 0, 'y', 0,
+ 'b', 0, 'r', 0, 'i', 0,
'd', 0, ' ', 0,
+ 'I', 0, 'S', 0, 'O', 0,
0, 0};
+ char part_name_bootimg[]= {'I', 0, 'S', 0, 'O', 0,
'H', 0, 'y', 0,
+ 'b', 0, 'r', 0, 'i', 0,
'd', 0, 0, 0};
if (mac_lba) {
/* 2048 bytes per partition, plus round to 2048 boundary */
@@ -793,7 +798,7 @@ initialise_gpt(uint8_t *gpt, uint32_t cu
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(0);
part->lastLBA = lendian_64(psize);
- memcpy(part->name, "ISOHybrid ISO", 28);
+ memcpy(part->name, part_name_iso, 28);
gpt += sizeof(struct gpt_part_header);
part++;
@@ -802,7 +807,7 @@ initialise_gpt(uint8_t *gpt, uint32_t cu
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(efi_lba * 4);
part->lastLBA = lendian_64(part->firstLBA + efi_count - 1);
- memcpy(part->name, "ISOHybrid", 20);
+ memcpy(part->name, part_name_bootimg, 20);
gpt += sizeof(struct gpt_part_header);
@@ -815,7 +820,7 @@ initialise_gpt(uint8_t *gpt, uint32_t cu
memcpy(part->partTypeGUID, hfs_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(mac_lba * 4);
part->lastLBA = lendian_64(part->firstLBA + mac_count - 1);
- memcpy(part->name, "ISOHybrid", 20);
+ memcpy(part->name, part_name_bootimg, 20);
part--;
}
Thomas Schmitt
2014-Jun-22 19:15 UTC
[syslinux] [PATCH] utils/isohybrid.c: 002 Correct blocking factor in APM partition block counts
The block counts in the APM partitions assumed 512 bytes per block,
whereas the start block numbers assume 2048 as announced in the APM header.
This change divides the affected block counts by 4 to correct the assumption.
--- isohybrid.c.001_gpt_name 2014-06-22 20:04:30.000000000 +0200
+++ isohybrid.c.002_apm_block_count 2014-06-22 20:05:13.000000000 +0200
@@ -842,7 +842,7 @@ initialise_apm(uint8_t *gpt, uint32_t st
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(apm_parts);
part->start_block = bendian_int(1);
- part->block_count = bendian_int(0x10);
+ part->block_count = bendian_int(4);
strcpy(part->name, "Apple");
strcpy(part->type, "Apple_partition_map");
part->data_start = bendian_int(0);
@@ -854,11 +854,11 @@ initialise_apm(uint8_t *gpt, uint32_t st
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(3);
part->start_block = bendian_int(efi_lba);
- part->block_count = bendian_int(efi_count);
+ part->block_count = bendian_int(efi_count / 4);
strcpy(part->name, "EFI");
strcpy(part->type, "Apple_HFS");
part->data_start = bendian_int(0);
- part->data_count = bendian_int(efi_count);
+ part->data_count = bendian_int(efi_count / 4);
part->status = bendian_int(0x33);
part = (struct apple_part_header *)(gpt + 4096);
@@ -868,11 +868,11 @@ initialise_apm(uint8_t *gpt, uint32_t st
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(3);
part->start_block = bendian_int(mac_lba);
- part->block_count = bendian_int(mac_count);
+ part->block_count = bendian_int(mac_count / 4);
strcpy(part->name, "EFI");
strcpy(part->type, "Apple_HFS");
part->data_start = bendian_int(0);
- part->data_count = bendian_int(mac_count);
+ part->data_count = bendian_int(mac_count / 4);
part->status = bendian_int(0x33);
} else {
part->signature = bendian_short(0x504d);
Thomas Schmitt
2014-Jun-22 19:27 UTC
[syslinux] [PATCH] utils/isohybrid.c: 003 Correct end block address of first GPT partition
The GPT partition 1 covers the whole ISO filesystem size. GPT specs
demand that the partition end block number shall be the last valid
block in the partition. isohybrid.c rather wrote the number of the
first block after the partition end.
This change reduces the number by 1.
--- isohybrid.c.002_apm_block_count 2014-06-22 20:05:13.000000000 +0200
+++ isohybrid.c.003_gpt_part_1_last_lba 2014-06-22 20:05:03.000000000 +0200
@@ -797,7 +797,7 @@ initialise_gpt(uint8_t *gpt, uint32_t cu
memcpy(part->partGUID, iso_uuid, sizeof(uuid_t));
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(0);
- part->lastLBA = lendian_64(psize);
+ part->lastLBA = lendian_64(psize - 1);
memcpy(part->name, part_name_iso, 28);
gpt += sizeof(struct gpt_part_header);
Thomas Schmitt
2014-Jun-22 19:33 UTC
[syslinux] [PATCH] utils/isohybrid.c: 004 Write GPT backup to the very end of the image
The GPT backup header block should start 512 bytes before the end of the image file (resp. end of the disk device). This block and the backup GPT array were wrongly written 512 bytes too early. This change brings the backup GPT at its correct position. --- isohybrid.c.003_gpt_part_1_last_lba 2014-06-22 20:05:03.000000000 +0200 +++ isohybrid.c.004_gpt_backup_adr 2014-06-22 20:09:06.000000000 +0200 @@ -1084,7 +1084,7 @@ main(int argc, char *argv[]) * Primary GPT starts at sector 1, secondary GPT starts at 1 sector * before the end of the image */ - initialise_gpt(buf, 1, (isostat.st_size + padding - 1024) / 512, 1); + initialise_gpt(buf, 1, (isostat.st_size + padding - 512) / 512, 1); if (fseek(fp, 512, SEEK_SET)) err(1, "%s: seek error - 6", argv[0]); @@ -1122,7 +1122,7 @@ main(int argc, char *argv[]) buf += orig_gpt_size - sizeof(struct gpt_header); - initialise_gpt(buf, (isostat.st_size + padding - 1024) / 512, 1, 0); + initialise_gpt(buf, (isostat.st_size + padding - 512) / 512, 1, 0); /* Shift back far enough to write the 128 GPT entries */ buf -= 128 * sizeof(struct gpt_part_header); @@ -1132,7 +1132,7 @@ main(int argc, char *argv[]) * end of the image */ - if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size - 512, + if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size, SEEK_SET)) err(1, "%s: seek error - 8", argv[0]);
Thomas Schmitt
2014-Jun-22 19:40 UTC
[syslinux] [PATCH] utils/isohybrid.c: 005 Change all fseek(3) to fseeko(3)
It seems unwise to offer future programmers fseek(3) calls for copy+paste.
They are simply insufficient for large image files.
This change switches all calls of fseek(3) to fseeko(3) and takes care
that the offset value if of type off_t.
--- isohybrid.c.004_gpt_backup_adr 2014-06-22 20:09:06.000000000 +0200
+++ isohybrid.c.005_fseeko 2014-06-22 20:09:01.000000000 +0200
@@ -916,13 +916,13 @@ main(int argc, char *argv[])
if (!(fp = fopen(argv[0], "r+")))
err(1, "could not open file `%s'", argv[0]);
- if (fseek(fp, (16 << 11), SEEK_SET))
+ if (fseeko(fp, (off_t) (16 << 11), SEEK_SET))
err(1, "%s: seek error - 0", argv[0]);
if (fread(&descriptor, sizeof(char), sizeof(descriptor), fp) !=
sizeof(descriptor))
err(1, "%s: read error - 0", argv[0]);
- if (fseek(fp, 17 * 2048, SEEK_SET))
+ if (fseeko(fp, (off_t) 17 * 2048, SEEK_SET))
err(1, "%s: seek error - 1", argv[0]);
bufz = buf = calloc(BUFSIZE, sizeof(char));
@@ -935,7 +935,7 @@ main(int argc, char *argv[])
if (mode & VERBOSE)
printf("catalogue offset: %d\n", catoffset);
- if (fseek(fp, catoffset * 2048, SEEK_SET))
+ if (fseeko(fp, ((off_t) catoffset) * 2048, SEEK_SET))
err(1, "%s: seek error - 2", argv[0]);
buf = bufz;
@@ -985,7 +985,7 @@ main(int argc, char *argv[])
}
}
- if (fseek(fp, (de_lba * 2048 + 0x40), SEEK_SET))
+ if (fseeko(fp, (((off_t) de_lba) * 2048 + 0x40), SEEK_SET))
err(1, "%s: seek error - 3", argv[0]);
buf = bufz;
@@ -1021,7 +1021,7 @@ main(int argc, char *argv[])
if (!id)
{
- if (fseek(fp, 440, SEEK_SET))
+ if (fseeko(fp, (off_t) 440, SEEK_SET))
err(1, "%s: seek error - 4", argv[0]);
if (fread(&id, 1, 4, fp) != 4)
@@ -1045,7 +1045,7 @@ main(int argc, char *argv[])
if (mode & VERBOSE)
display_mbr(buf, i);
- if (fseek(fp, 0, SEEK_SET))
+ if (fseeko(fp, (off_t) 0, SEEK_SET))
err(1, "%s: seek error - 5", argv[0]);
if (fwrite(buf, sizeof(char), i, fp) != (size_t)i)
@@ -1086,7 +1086,7 @@ main(int argc, char *argv[])
*/
initialise_gpt(buf, 1, (isostat.st_size + padding - 512) / 512, 1);
- if (fseek(fp, 512, SEEK_SET))
+ if (fseeko(fp, (off_t) 512, SEEK_SET))
err(1, "%s: seek error - 6", argv[0]);
if (fwrite(buf, sizeof(char), gpt_size, fp) != (size_t)gpt_size)
@@ -1103,7 +1103,7 @@ main(int argc, char *argv[])
initialise_apm(buf, APM_OFFSET);
- fseek(fp, APM_OFFSET, SEEK_SET);
+ fseeko(fp, (off_t) APM_OFFSET, SEEK_SET);
fwrite(buf, sizeof(char), apm_size, fp);
}
@@ -1132,8 +1132,7 @@ main(int argc, char *argv[])
* end of the image
*/
- if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size,
- SEEK_SET))
+ if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size, SEEK_SET))
err(1, "%s: seek error - 8", argv[0]);
if (fwrite(buf, sizeof(char), orig_gpt_size, fp) != orig_gpt_size)
Thomas Schmitt
2014-Jun-22 19:58 UTC
[syslinux] [PATCH] utils/isohybrid.c: 006 Introduce option --mbr and make isohybrid.c compilable standalone
Although isohybrid.c is supposed to be a companion of the local SYSLINUX
installation, there may be situations where the file isolinux.bin and the
matching MBR template do not stem directly from such an installation.
This change adds an option --mbr, which allows to load an MBR template
file. This may be an isohdp[fp]x*.bin MBR template from the local SYSLINUX
installation, or the first 512 bytes of the isohybrid-capable ISO image
from which isolinux.bin and the other ISOLINUX files are taken.
If macro ISOHYBRID_C_STANDALONE is defined, then the hardcoded MBR templates
are not accessible and isohdpfx.o is not needed at compile time. In this
case, option --mbr becomes mandatory.
I used this for testing my changes with Fedora-Live-Desktop-x86_64-20-1.iso.
isohybrid.c is then compilable without further components of ISOLINUX by:
cc -DISOHYBRID_C_STANDALONE -Wall -o isohybrid isohybrid.c -luuid
Test run:
cp Fedora-Live-Desktop-x86_64-20-1.iso \
Fedora-Live-Desktop-x86_64-20-1-rehybrid.iso
valgrind ./isohybrid --uefi --mac \
--mbr Fedora-Live-Desktop-x86_64-20-1.mbr \
Fedora-Live-Desktop-x86_64-20-1-rehybrid.iso
yields:
==13828== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 1)
...
==13828== LEAK SUMMARY:
==13828== definitely lost: 2,048 bytes in 1 blocks.
(Not that valgrind would have detected the memcpy() abuse of patch 001.
But at least i seem to have not introduced more obvious sins.)
--- isohybrid.c.005_fseeko 2014-06-22 20:09:01.000000000 +0200
+++ isohybrid.c.006_opt_mbr_standalone 2014-06-22 20:10:49.000000000 +0200
@@ -63,6 +63,8 @@ uint32_t id = 0; /* MBR:
uint8_t hd0 = 0; /* 0 <= hd0 <= 2 */
uint8_t partok = 0; /* 0 <= partok <= 1 */
+char mbr_template_path[1024] = {0}; /* Path to MBR template */
+
uint16_t ve[16];
uint32_t catoffset = 0;
uint32_t c = 0, cc = 0, cs = 0;
@@ -223,7 +225,7 @@ usage(void)
void
printh(void)
{
-#define FMT "%-18s %s\n"
+#define FMT "%-20s %s\n"
usage();
@@ -237,6 +239,7 @@ printh(void)
printf(FMT, " -i --id", "Specify MBR ID (default
random)");
printf(FMT, " -u --uefi", "Build EFI bootable
image");
printf(FMT, " -m --mac", "Add AFP table support");
+ printf(FMT, " -b --mbr <PATH>", "Load MBR from
PATH");
printf("\n");
printf(FMT, " --forcehd0", "Assume we are loaded as disk
ID 0");
@@ -272,6 +275,7 @@ check_option(int argc, char *argv[])
{ "partok", no_argument, NULL, 'p'},
{ "uefi", no_argument, NULL, 'u'},
{ "mac", no_argument, NULL, 'm'},
+ { "mbr", required_argument, NULL, 'b' },
{ "help", no_argument, NULL, '?' },
{ "verbose", no_argument, NULL, 'v' },
@@ -347,6 +351,12 @@ check_option(int argc, char *argv[])
errx(1, "setting an entry is unsupported with EFI or Mac");
break;
+ case 'b':
+ if (strlen(optarg) >= sizeof(mbr_template_path))
+ errx(1, "--mbr : Path too long");
+ strcpy(mbr_template_path, optarg);
+ break;
+
case 'v':
mode |= VERBOSE;
break;
@@ -571,6 +581,24 @@ display_catalogue(void)
printf("de_mbz2: %hu\n", de_mbz2);
}
+
+void
+read_mbr_template(char *path, uint8_t *mbr)
+{
+ FILE *fp;
+ int ret;
+
+ fp = fopen(path, "rb");
+ if (fp == NULL)
+ err(1, "could not open MBR template file `%s'", path);
+ clearerr(fp);
+ ret = fread(mbr, 1, MBRSIZE, fp);
+ if (ferror(fp))
+ err(1, "error while reading MBR template file `%s'",
path);
+ fclose(fp);
+}
+
+
int
initialise_mbr(uint8_t *mbr)
{
@@ -580,9 +608,25 @@ initialise_mbr(uint8_t *mbr)
uint8_t bhead = 0, bsect = 0, bcyle = 0;
uint8_t ehead = 0, esect = 0, ecyle = 0;
+#ifndef ISOHYBRID_C_STANDALONE
extern unsigned char isohdpfx[][MBRSIZE];
+#endif
+
+ if (mbr_template_path[0]) {
+ read_mbr_template(mbr_template_path, mbr);
+ } else {
+
+#ifdef ISOHYBRID_C_STANDALONE
+
+ err(1, "This is a standalone binary. You must specify --mbr. E.g
with /usr/lib/syslinux/isohdpfx.bin");
- memcpy(mbr, &isohdpfx[hd0 + 3 * partok], MBRSIZE);
+#else
+
+ memcpy(mbr, &isohdpfx[hd0 + 3 * partok], MBRSIZE);
+
+#endif /* ! ISOHYBRID_C_STANDALONE */
+
+ }
if (mode & MAC) {
memcpy(mbr, afp_header, sizeof(afp_header));
Thomas Schmitt
2014-Jun-22 20:21 UTC
[syslinux] [PATCH] utils/isohybrid.c: 007 Enable promised options -u, -m, -b
It turned out that i copied the options handling from suboptimal
example code.
This change enables the options -u, -m, -b as promised by the help text.
--- isohybrid.c.006_opt_mbr_standalone 2014-06-22 20:10:49.000000000 +0200
+++ isohybrid.c.007_short_opts_umb 2014-06-22 22:14:03.000000000 +0200
@@ -262,7 +262,7 @@ check_option(int argc, char *argv[])
char *err = NULL;
int n = 0, ind = 0;
- const char optstr[] = ":h:s:e:o:t:i:fcp?vV";
+ const char optstr[] = ":h:s:e:o:t:i:u:m:b:fcp?vV";
struct option lopt[] = \
{
{ "entry", required_argument, NULL, 'e' },
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 1/6] utils/isohybrid.c: Encode GPT partition names as UTF-16LE
The worst sin of isohybrid.c was to compose GPT partition names by
8-bit characters and to memcpy() them as if they were 16 bit wide.
GPT names are encoded as UTF-16LE. It is trivial to create this encoding
from 7-bit ASCII.
This change introduces two byte arrays with the desired UTF-16LE names
which replace the string constants "ISOHybrid ISO" and
"ISOHybrid".
---
utils/isohybrid.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index 05afd29..c5b4281 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -749,6 +749,11 @@ initialise_gpt(uint8_t *gpt, uint32_t current, uint32_t
alternate, int primary)
struct gpt_part_header *part;
int hole = 0;
int gptsize = 128 / 4 + 2;
+ char part_name_iso[] = {'I', 0, 'S', 0, 'O', 0,
'H', 0, 'y', 0,
+ 'b', 0, 'r', 0, 'i', 0,
'd', 0, ' ', 0,
+ 'I', 0, 'S', 0, 'O', 0,
0, 0};
+ char part_name_bootimg[]= {'I', 0, 'S', 0, 'O', 0,
'H', 0, 'y', 0,
+ 'b', 0, 'r', 0, 'i', 0,
'd', 0, 0, 0};
if (mac_lba) {
/* 2048 bytes per partition, plus round to 2048 boundary */
@@ -793,7 +798,7 @@ initialise_gpt(uint8_t *gpt, uint32_t current, uint32_t
alternate, int primary)
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(0);
part->lastLBA = lendian_64(psize);
- memcpy(part->name, "ISOHybrid ISO", 28);
+ memcpy(part->name, part_name_iso, 28);
gpt += sizeof(struct gpt_part_header);
part++;
@@ -802,7 +807,7 @@ initialise_gpt(uint8_t *gpt, uint32_t current, uint32_t
alternate, int primary)
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(efi_lba * 4);
part->lastLBA = lendian_64(part->firstLBA + efi_count - 1);
- memcpy(part->name, "ISOHybrid", 20);
+ memcpy(part->name, part_name_bootimg, 20);
gpt += sizeof(struct gpt_part_header);
@@ -815,7 +820,7 @@ initialise_gpt(uint8_t *gpt, uint32_t current, uint32_t
alternate, int primary)
memcpy(part->partTypeGUID, hfs_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(mac_lba * 4);
part->lastLBA = lendian_64(part->firstLBA + mac_count - 1);
- memcpy(part->name, "ISOHybrid", 20);
+ memcpy(part->name, part_name_bootimg, 20);
part--;
}
--
1.8.4.2
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 2/6] utils/isohybrid.c: Correct blocking factor in APM partition block counts
The block counts in the APM partitions assumed 512 bytes per block,
whereas the start block numbers assume 2048 as announced in the APM header.
This change divides the affected block counts by 4 to correct the assumption.
---
utils/isohybrid.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index c5b4281..7d0864e 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -842,7 +842,7 @@ initialise_apm(uint8_t *gpt, uint32_t start)
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(apm_parts);
part->start_block = bendian_int(1);
- part->block_count = bendian_int(0x10);
+ part->block_count = bendian_int(4);
strcpy(part->name, "Apple");
strcpy(part->type, "Apple_partition_map");
part->data_start = bendian_int(0);
@@ -854,11 +854,11 @@ initialise_apm(uint8_t *gpt, uint32_t start)
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(3);
part->start_block = bendian_int(efi_lba);
- part->block_count = bendian_int(efi_count);
+ part->block_count = bendian_int(efi_count / 4);
strcpy(part->name, "EFI");
strcpy(part->type, "Apple_HFS");
part->data_start = bendian_int(0);
- part->data_count = bendian_int(efi_count);
+ part->data_count = bendian_int(efi_count / 4);
part->status = bendian_int(0x33);
part = (struct apple_part_header *)(gpt + 4096);
@@ -868,11 +868,11 @@ initialise_apm(uint8_t *gpt, uint32_t start)
part->signature = bendian_short(0x504d);
part->map_count = bendian_int(3);
part->start_block = bendian_int(mac_lba);
- part->block_count = bendian_int(mac_count);
+ part->block_count = bendian_int(mac_count / 4);
strcpy(part->name, "EFI");
strcpy(part->type, "Apple_HFS");
part->data_start = bendian_int(0);
- part->data_count = bendian_int(mac_count);
+ part->data_count = bendian_int(mac_count / 4);
part->status = bendian_int(0x33);
} else {
part->signature = bendian_short(0x504d);
--
1.8.4.2
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 3/6] utils/isohybrid.c: Correct end block address of first GPT partition
The GPT partition 1 covers the whole ISO filesystem size. GPT specs
demand that the partition end block number shall be the last valid
block in the partition. isohybrid.c rather wrote the number of the
first block after the partition end.
This change reduces the number by 1.
---
utils/isohybrid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index 7d0864e..ff6b930 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -797,7 +797,7 @@ initialise_gpt(uint8_t *gpt, uint32_t current, uint32_t
alternate, int primary)
memcpy(part->partGUID, iso_uuid, sizeof(uuid_t));
memcpy(part->partTypeGUID, basic_partition, sizeof(uuid_t));
part->firstLBA = lendian_64(0);
- part->lastLBA = lendian_64(psize);
+ part->lastLBA = lendian_64(psize - 1);
memcpy(part->name, part_name_iso, 28);
gpt += sizeof(struct gpt_part_header);
--
1.8.4.2
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 4/6] utils/isohybrid.c: Write GPT backup to the very end of the image
The GPT backup header block should start 512 bytes before the end of the image file (resp. end of the disk device). This block and the backup GPT array were wrongly written 512 bytes too early. This change brings the backup GPT at its correct position. --- utils/isohybrid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/isohybrid.c b/utils/isohybrid.c index ff6b930..1c8f0b6 100644 --- a/utils/isohybrid.c +++ b/utils/isohybrid.c @@ -1084,7 +1084,7 @@ main(int argc, char *argv[]) * Primary GPT starts at sector 1, secondary GPT starts at 1 sector * before the end of the image */ - initialise_gpt(buf, 1, (isostat.st_size + padding - 1024) / 512, 1); + initialise_gpt(buf, 1, (isostat.st_size + padding - 512) / 512, 1); if (fseek(fp, 512, SEEK_SET)) err(1, "%s: seek error - 6", argv[0]); @@ -1122,7 +1122,7 @@ main(int argc, char *argv[]) buf += orig_gpt_size - sizeof(struct gpt_header); - initialise_gpt(buf, (isostat.st_size + padding - 1024) / 512, 1, 0); + initialise_gpt(buf, (isostat.st_size + padding - 512) / 512, 1, 0); /* Shift back far enough to write the 128 GPT entries */ buf -= 128 * sizeof(struct gpt_part_header); @@ -1132,7 +1132,7 @@ main(int argc, char *argv[]) * end of the image */ - if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size - 512, + if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size, SEEK_SET)) err(1, "%s: seek error - 8", argv[0]); -- 1.8.4.2
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 5/6] utils/isohybrid.c: Change all fseek(3) to fseeko(3)
It seems unwise to offer future programmers fseek(3) calls for copy+paste.
They are simply insufficient for large image files.
This change switches all calls of fseek(3) to fseeko(3) and takes care
that the offset value if of type off_t.
---
utils/isohybrid.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index 1c8f0b6..072402f 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -916,13 +916,13 @@ main(int argc, char *argv[])
if (!(fp = fopen(argv[0], "r+")))
err(1, "could not open file `%s'", argv[0]);
- if (fseek(fp, (16 << 11), SEEK_SET))
+ if (fseeko(fp, (off_t) (16 << 11), SEEK_SET))
err(1, "%s: seek error - 0", argv[0]);
if (fread(&descriptor, sizeof(char), sizeof(descriptor), fp) !=
sizeof(descriptor))
err(1, "%s: read error - 0", argv[0]);
- if (fseek(fp, 17 * 2048, SEEK_SET))
+ if (fseeko(fp, (off_t) 17 * 2048, SEEK_SET))
err(1, "%s: seek error - 1", argv[0]);
bufz = buf = calloc(BUFSIZE, sizeof(char));
@@ -935,7 +935,7 @@ main(int argc, char *argv[])
if (mode & VERBOSE)
printf("catalogue offset: %d\n", catoffset);
- if (fseek(fp, catoffset * 2048, SEEK_SET))
+ if (fseeko(fp, ((off_t) catoffset) * 2048, SEEK_SET))
err(1, "%s: seek error - 2", argv[0]);
buf = bufz;
@@ -985,7 +985,7 @@ main(int argc, char *argv[])
}
}
- if (fseek(fp, (de_lba * 2048 + 0x40), SEEK_SET))
+ if (fseeko(fp, (((off_t) de_lba) * 2048 + 0x40), SEEK_SET))
err(1, "%s: seek error - 3", argv[0]);
buf = bufz;
@@ -1021,7 +1021,7 @@ main(int argc, char *argv[])
if (!id)
{
- if (fseek(fp, 440, SEEK_SET))
+ if (fseeko(fp, (off_t) 440, SEEK_SET))
err(1, "%s: seek error - 4", argv[0]);
if (fread(&id, 1, 4, fp) != 4)
@@ -1045,7 +1045,7 @@ main(int argc, char *argv[])
if (mode & VERBOSE)
display_mbr(buf, i);
- if (fseek(fp, 0, SEEK_SET))
+ if (fseeko(fp, (off_t) 0, SEEK_SET))
err(1, "%s: seek error - 5", argv[0]);
if (fwrite(buf, sizeof(char), i, fp) != (size_t)i)
@@ -1086,7 +1086,7 @@ main(int argc, char *argv[])
*/
initialise_gpt(buf, 1, (isostat.st_size + padding - 512) / 512, 1);
- if (fseek(fp, 512, SEEK_SET))
+ if (fseeko(fp, (off_t) 512, SEEK_SET))
err(1, "%s: seek error - 6", argv[0]);
if (fwrite(buf, sizeof(char), gpt_size, fp) != (size_t)gpt_size)
@@ -1103,7 +1103,7 @@ main(int argc, char *argv[])
initialise_apm(buf, APM_OFFSET);
- fseek(fp, APM_OFFSET, SEEK_SET);
+ fseeko(fp, (off_t) APM_OFFSET, SEEK_SET);
fwrite(buf, sizeof(char), apm_size, fp);
}
@@ -1132,8 +1132,7 @@ main(int argc, char *argv[])
* end of the image
*/
- if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size,
- SEEK_SET))
+ if (fseeko(fp, (isostat.st_size + padding) - orig_gpt_size, SEEK_SET))
err(1, "%s: seek error - 8", argv[0]);
if (fwrite(buf, sizeof(char), orig_gpt_size, fp) != orig_gpt_size)
--
1.8.4.2
Thomas Schmitt
2014-Jun-22 20:24 UTC
[syslinux] [PATCH 6/6] utils/isohybrid.c: Introduce option --mbr and make isohybrid.c compilable standalone
Although isohybrid.c is supposed to be a companion of the local SYSLINUX
installation, there may be situations where the file isolinux.bin and the
matching MBR template do not stem directly from such an installation.
This change adds an option --mbr, which allows to load an MBR template
file. This may be an isohdp[fp]x*.bin MBR template from the local SYSLINUX
installation, or the first 512 bytes of the isohybrid-capable ISO image
from which isolinux.bin and the other ISOLINUX files are taken.
If macro ISOHYBRID_C_STANDALONE is defined, then the hardcoded MBR templates
are not accessible and isohdpfx.o is not needed at compile time. In this
case, option --mbr becomes mandatory.
I used this for testing my changes with Fedora-Live-Desktop-x86_64-20-1.iso.
isohybrid.c is then compilable without further components of ISOLINUX by:
cc -DISOHYBRID_C_STANDALONE -Wall -o isohybrid isohybrid.c -luuid
Test run:
cp Fedora-Live-Desktop-x86_64-20-1.iso \
Fedora-Live-Desktop-x86_64-20-1-rehybrid.iso
valgrind ./isohybrid --uefi --mac \
--mbr Fedora-Live-Desktop-x86_64-20-1.mbr \
Fedora-Live-Desktop-x86_64-20-1-rehybrid.iso
yields:
==13828== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 1)
...
==13828== LEAK SUMMARY:
==13828== definitely lost: 2,048 bytes in 1 blocks.
(Not that valgrind would have detected the memcpy() abuse of patch 001.
But at least i seem to have not introduced more obvious sins.)
---
utils/isohybrid.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index 072402f..0011b78 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -63,6 +63,8 @@ uint32_t id = 0; /* MBR: 0 <= id <=
0xFFFFFFFF(4294967296) */
uint8_t hd0 = 0; /* 0 <= hd0 <= 2 */
uint8_t partok = 0; /* 0 <= partok <= 1 */
+char mbr_template_path[1024] = {0}; /* Path to MBR template */
+
uint16_t ve[16];
uint32_t catoffset = 0;
uint32_t c = 0, cc = 0, cs = 0;
@@ -223,7 +225,7 @@ usage(void)
void
printh(void)
{
-#define FMT "%-18s %s\n"
+#define FMT "%-20s %s\n"
usage();
@@ -237,6 +239,7 @@ printh(void)
printf(FMT, " -i --id", "Specify MBR ID (default
random)");
printf(FMT, " -u --uefi", "Build EFI bootable
image");
printf(FMT, " -m --mac", "Add AFP table support");
+ printf(FMT, " -b --mbr <PATH>", "Load MBR from
PATH");
printf("\n");
printf(FMT, " --forcehd0", "Assume we are loaded as disk
ID 0");
@@ -272,6 +275,7 @@ check_option(int argc, char *argv[])
{ "partok", no_argument, NULL, 'p'},
{ "uefi", no_argument, NULL, 'u'},
{ "mac", no_argument, NULL, 'm'},
+ { "mbr", required_argument, NULL, 'b' },
{ "help", no_argument, NULL, '?' },
{ "verbose", no_argument, NULL, 'v' },
@@ -347,6 +351,12 @@ check_option(int argc, char *argv[])
errx(1, "setting an entry is unsupported with EFI or Mac");
break;
+ case 'b':
+ if (strlen(optarg) >= sizeof(mbr_template_path))
+ errx(1, "--mbr : Path too long");
+ strcpy(mbr_template_path, optarg);
+ break;
+
case 'v':
mode |= VERBOSE;
break;
@@ -571,6 +581,24 @@ display_catalogue(void)
printf("de_mbz2: %hu\n", de_mbz2);
}
+
+void
+read_mbr_template(char *path, uint8_t *mbr)
+{
+ FILE *fp;
+ int ret;
+
+ fp = fopen(path, "rb");
+ if (fp == NULL)
+ err(1, "could not open MBR template file `%s'", path);
+ clearerr(fp);
+ ret = fread(mbr, 1, MBRSIZE, fp);
+ if (ferror(fp))
+ err(1, "error while reading MBR template file `%s'",
path);
+ fclose(fp);
+}
+
+
int
initialise_mbr(uint8_t *mbr)
{
@@ -580,9 +608,25 @@ initialise_mbr(uint8_t *mbr)
uint8_t bhead = 0, bsect = 0, bcyle = 0;
uint8_t ehead = 0, esect = 0, ecyle = 0;
+#ifndef ISOHYBRID_C_STANDALONE
extern unsigned char isohdpfx[][MBRSIZE];
+#endif
+
+ if (mbr_template_path[0]) {
+ read_mbr_template(mbr_template_path, mbr);
+ } else {
+
+#ifdef ISOHYBRID_C_STANDALONE
+
+ err(1, "This is a standalone binary. You must specify --mbr. E.g
with /usr/lib/syslinux/isohdpfx.bin");
- memcpy(mbr, &isohdpfx[hd0 + 3 * partok], MBRSIZE);
+#else
+
+ memcpy(mbr, &isohdpfx[hd0 + 3 * partok], MBRSIZE);
+
+#endif /* ! ISOHYBRID_C_STANDALONE */
+
+ }
if (mode & MAC) {
memcpy(mbr, afp_header, sizeof(afp_header));
--
1.8.4.2
Op 2014-06-22 om 21:02 schreef Thomas Schmitt:> Hi, > > following will be 6 patch proposals for isohybrid.c > > 1: Encode GPT partition names as UTF-16LE > > 2: Correct blocking factor in APM partition block counts > > 3: Correct end block address of first GPT partition > > 4: Write GPT backup to the very end of the image > > 5: Change all fseek(3) to fseeko(3) > > 6: Introduce option --mbr and make isohybrid.c compilable standalone > > If the form needs adjustments, i whould be able to reproduce the > patches.Those patches regenerated from git are send to Thomas and the ML. We will see is they go a cross various e-mail (spam) filters.> The state after patch 6 was tested by re-hybdridization of > yetserday's Fedora-Live-Desktop-x86_64-20-1.iso. > The result was loaded by xorriso which then assessed and > reported the MBR, GPT, and APM equipment. > > One false positive complaint appears: > GPT backup problems: Implausible header LBA 1951743, Implausible array LBA 1951711 > It could be avoided by adjusting the ISO filesystem size > to the size of the image file. Regrettably this causes > isohybrid.c to add another MiB of padding if the image > is isohybridized again. > So i will ponder how to recognize the habit of isohybrid.c > as a tolerable deviation - or how to let isohybrid.c recognize > that the filesystem already has a backup GPT at its end, > so that no new space is needed.Please do.> Have a nice day :) > > ThomasGroeten Geert Stappers -- Leven en laten leven
Op 2014-06-22 om 22:31 schreef Geert Stappers:> Op 2014-06-22 om 21:02 schreef Thomas Schmitt: > > > > following will be 6 patch proposals for isohybrid.c > > > > 1: Encode GPT partition names as UTF-16LE > > 2: Correct blocking factor in APM partition block counts > > 3: Correct end block address of first GPT partition > > 4: Write GPT backup to the very end of the image > > 5: Change all fseek(3) to fseeko(3) > > 6: Introduce option --mbr and make isohybrid.c compilable standalone > > > > If the form needs adjustments, i whould be able to reproduce the > > patches. > > Those patches regenerated from git are send to Thomas and the ML. > We will see is they go a cross various e-mail (spam) filters.We will see if they go a cross various e-mail (spam) filters. They did all six. There was a seventh patch. It uses also reformatting. Don't wait for me do it. Groeten Geert Stappers -- Leven en laten leven
Thomas Schmitt
2014-Jun-23 17:57 UTC
[syslinux] [PATCH] utils/isohybrid.c: 007 (2nd try) Enable promised options -u, -m, -b
My first attempt ro enable the short options for --uefi, --mac, --mbr
was faulty. Here is the second try.
This change enables the options -u, -m, -b as promised by the help text.
--- isohybrid.c.006_opt_mbr_standalone 2014-06-22 20:10:49.000000000 +0200
+++ isohybrid.c.007_short_opts_umb 2014-06-23 19:52:59.000000000 +0200
@@ -262,7 +262,7 @@ check_option(int argc, char *argv[])
char *err = NULL;
int n = 0, ind = 0;
- const char optstr[] = ":h:s:e:o:t:i:fcp?vV";
+ const char optstr[] = ":h:s:e:o:t:i:b:umfcp?vV";
struct option lopt[] = \
{
{ "entry", required_argument, NULL, 'e' },
Maybe Matching Threads
- isolinux: Generate GPT and Mac bootable images
- [PATCH 5/6] utils/isohybrid.c: Change all fseek(3) to fseeko(3)
- Announcing a patch series for isohybrid.c
- [PATCH 3/6] utils/isohybrid.c: Correct end block address of first GPT partition
- [PATCH 1/6] utils/isohybrid.c: Encode GPT partition names as UTF-16LE