From 443045b331582cd582da8433e3ff8c5a425a8112 Mon Sep 17 00:00:00 2001 From: "R. Timothy Edwards" Date: Mon, 6 Jul 2026 16:06:07 -0400 Subject: [PATCH] Fixed a known issue in which the output of "vendor" (read-only) GDS can become corrupted if the name of the cell in magic changes from the name of the structure in GDS pointed to by the GDS_FILE property, and the new name is a different length from the original name. Fixed by Claude Fable 5 and checked for all name-change scenarios for both compressed and uncompressed output. --- calma/CalmaWrite.c | 253 +++++++++++++++++++++++++++---------------- calma/CalmaWriteZ.c | 255 ++++++++++++++++++++++++++++---------------- calma/calmaInt.h | 3 + 3 files changed, 324 insertions(+), 187 deletions(-) diff --git a/calma/CalmaWrite.c b/calma/CalmaWrite.c index 045805f4..51918641 100644 --- a/calma/CalmaWrite.c +++ b/calma/CalmaWrite.c @@ -347,7 +347,7 @@ CalmaWrite( * to insure that each child cell is output before it is used. The * root cell is output last. */ - calmaProcessDef(rootDef, f, CalmaDoLibrary); + good = (calmaProcessDef(rootDef, f, CalmaDoLibrary) == 0) ? TRUE : FALSE; /* * Check for any cells that were instanced in the output definition @@ -366,7 +366,10 @@ CalmaWrite( extraDef = DBCellLookDef((char *)he->h_key.h_name); if (extraDef != NULL) - calmaProcessDef(extraDef, f, FALSE); + { + if (calmaProcessDef(extraDef, f, FALSE) != 0) + good = FALSE; + } else TxError("Error: Cell %s is not defined in the output file!\n", refname + 1); @@ -376,7 +379,7 @@ CalmaWrite( /* Finish up by outputting the end-of-library marker */ calmaOutRH(4, CALMA_ENDLIB, CALMA_NODATA, f); fflush(f); - good = !ferror(f); + if (ferror(f)) good = FALSE; /* See if any problems occurred */ if ((problems = (DBWFeedbackCount - oldCount))) @@ -975,11 +978,10 @@ calmaProcessDef( if (isReadOnly && hasContent) { - char *buffer, *offptr, *retfilename; + char *buffer, *retfilename; size_t defsize, numbytes; - off_t cellstart, cellend, structstart; + off_t cellstart, cellend; dlong cval; - int namelen; FILETYPE fi; /* Give some feedback to the user */ @@ -1039,72 +1041,109 @@ calmaProcessDef( cellend = (off_t)cval; cval = DBPropGetDouble(def, "GDS_BEGIN", &oldStyle); if (!oldStyle) - { cval = DBPropGetDouble(def, "GDS_START", NULL); - /* Write our own header and string name, to ensure */ - /* that the magic cell name and GDS name match. */ - - /* Output structure header */ - calmaOutRH(28, CALMA_BGNSTR, CALMA_I2, outf); - if (CalmaDateStamp != NULL) - calmaOutDate(*CalmaDateStamp, outf); - else - calmaOutDate(def->cd_timestamp, outf); - calmaOutDate(time((time_t *) 0), outf); - - /* Name structure the same as the magic cellname */ - calmaOutStructName(CALMA_STRNAME, def, outf); - } - cellstart = (off_t)cval; - /* GDS_START has been defined as the start of data after the cell */ - /* structure name. However, a sanity check is wise, so move back */ - /* that far and make sure the structure name is there. */ - structstart = cellstart - (off_t)(strlen(def->cd_name)); - if (strlen(def->cd_name) & 0x1) structstart--; - structstart -= 2; + /* Sanity check: The structure must be long enough to */ + /* hold at least an ENDSTR record. */ - FSEEK(fi, structstart, SEEK_SET); - - /* Read the structure name and check against the CellDef name */ - defsize = (size_t)(cellstart - structstart); - buffer = (char *)mallocMagic(defsize + 1); - numbytes = magicFREAD(buffer, sizeof(char), (size_t)defsize, fi); - if (numbytes == defsize) - { - buffer[defsize] = '\0'; - if (buffer[0] != 0x06 || buffer[1] != 0x06) - { - TxError("Calma output error: Structure name not found" - " at GDS file position %"DLONG_PREFIX"d\n", cellstart); - TxError("Calma output error: Can't write cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - else if (strcmp(&buffer[2], def->cd_name)) - { - TxError("Calma output warning: Structure definition has name" - " %s but cell definition has name %s.\n", - &buffer[2], def->cd_name); - TxError("The structure definition will be given the cell name.\n"); - } - } - else - { - TxError("Calma output error: Can't read cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - freeMagic(buffer); - - if (cellend < cellstart) /* Sanity check */ + if (cellend < cellstart + 4) { TxError("Calma output error: Bad vendor GDS file reference!\n"); isReadOnly = FALSE; } + + if (isReadOnly && !oldStyle) + { + /* GDS_START has been defined as the start of data after the */ + /* cell structure name. A sanity check is wise, so search */ + /* backward from that position for the STRNAME record header. */ + /* Note that the length of the structure name in the file */ + /* cannot be assumed to be the length of the cell name in */ + /* magic, because the cell may have been renamed after the */ + /* GDS file was read. */ + + off_t blockstart; + size_t blocksize; + int reclen; + char *sptr = NULL; + + blockstart = cellstart - (off_t)CALMARECORDMAX; + if (blockstart < 0) blockstart = (off_t)0; + blocksize = (size_t)(cellstart - blockstart); + + FSEEK(fi, blockstart, SEEK_SET); + buffer = (char *)mallocMagic(blocksize + 1); + numbytes = magicFREAD(buffer, sizeof(char), blocksize, fi); + if (numbytes == blocksize) + { + buffer[blocksize] = '\0'; + + /* Scan candidate record lengths from shortest to longest. */ + /* A structure name contains only printable characters, so */ + /* the first match found is the STRNAME record header. */ + + for (reclen = 6; reclen <= (int)blocksize; reclen += 2) + { + char *tptr = buffer + blocksize - reclen; + if (((unsigned char)tptr[0] == (unsigned char)(reclen >> 8)) + && ((unsigned char)tptr[1] == (unsigned char)(reclen & 0xff)) + && (tptr[2] == CALMA_STRNAME) + && (tptr[3] == CALMA_ASCII)) + { + sptr = tptr; + break; + } + } + if (sptr == NULL) + { + TxError("Calma output error: Structure name not found" + " ending at GDS file position %"DLONG_PREFIX"d\n", + (dlong)cellstart); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + else if (strcmp(sptr + 4, def->cd_name)) + { + TxError("Calma output warning: Structure definition has name" + " %s but cell definition has name %s.\n", + sptr + 4, def->cd_name); + TxError("The structure definition will be given the cell name.\n"); + } + } + else + { + TxError("Calma output error: Can't read cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + freeMagic(buffer); + } else if (isReadOnly) + { + /* Old-style GDS_BEGIN points to the start of the structure */ + /* (BGNSTR record), which is copied verbatim including the */ + /* structure name; check that the BGNSTR record is there. */ + + char header[4]; + + FSEEK(fi, cellstart, SEEK_SET); + numbytes = magicFREAD(header, sizeof(char), 4, fi); + if ((numbytes != 4) || (header[2] != CALMA_BGNSTR) + || (header[3] != CALMA_I2)) + { + TxError("Calma output error: Structure definition not found" + " at GDS file position %"DLONG_PREFIX"d\n", + (dlong)cellstart); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + } + + if (isReadOnly) { /* Important note: mallocMagic() is limited to size integer. */ /* This will fail on a structure larger than ~2GB. */ @@ -1112,46 +1151,74 @@ calmaProcessDef( defsize = (size_t)(cellend - cellstart); buffer = (char *)mallocMagic(defsize); + FSEEK(fi, cellstart, SEEK_SET); numbytes = magicFREAD(buffer, sizeof(char), (size_t)defsize, fi); - if (numbytes == defsize) - { - /* Sanity check: buffer must end with a structure */ - /* definition end (record 0x07). */ - - if (buffer[defsize - 4] != 0x00 || - buffer[defsize - 3] != 0x04 || - buffer[defsize - 2] != 0x07 || - buffer[defsize - 1] != 0x00) - { - TxError("Calma output error: Structure end definition not found" - " at GDS file position %"DLONG_PREFIX"d\n", cellend); - TxError("Calma output error: Can't write cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - else - { - numbytes = fwrite(buffer, sizeof(char), (size_t)defsize, outf); - if (numbytes <= 0) - { - TxError("Calma output error: Can't write cell from " - "vendor GDS. Using magic's internal " - "definition\n"); - isReadOnly = FALSE; - } - } - } - else + if (numbytes != defsize) { TxError("Calma output error: Can't read cell from vendor GDS." " Using magic's internal definition\n"); /* Additional information as to why data did not match */ - TxError("Size of data requested: %"DLONG_PREFIX"d", defsize); - TxError("Length of data read: %"DLONG_PREFIX"d", numbytes); + TxError("Size of data requested: %"DLONG_PREFIX"d", (dlong)defsize); + TxError("Length of data read: %"DLONG_PREFIX"d", (dlong)numbytes); isReadOnly = FALSE; } + else if (buffer[defsize - 4] != 0x00 || + buffer[defsize - 3] != 0x04 || + buffer[defsize - 2] != 0x07 || + buffer[defsize - 1] != 0x00) + { + /* Sanity check: buffer must end with a structure */ + /* definition end (record 0x07). */ + + TxError("Calma output error: Structure end definition not found" + " at GDS file position %"DLONG_PREFIX"d\n", cellend); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + else + { + /* All sanity checks have passed, so the structure header, */ + /* name, and data can be written. Nothing may be written */ + /* to the output before this point, so that a failed check */ + /* above falls back on the cell definition in magic without */ + /* leaving a partial structure in the output. */ + + if (!oldStyle) + { + /* Write our own header and string name, to ensure */ + /* that the magic cell name and GDS name match. */ + + /* Output structure header */ + calmaOutRH(28, CALMA_BGNSTR, CALMA_I2, outf); + if (CalmaDateStamp != NULL) + calmaOutDate(*CalmaDateStamp, outf); + else + calmaOutDate(def->cd_timestamp, outf); + calmaOutDate(time((time_t *) 0), outf); + + /* Name structure the same as the magic cellname */ + calmaOutStructName(CALMA_STRNAME, def, outf); + } + + numbytes = fwrite(buffer, sizeof(char), (size_t)defsize, outf); + if (numbytes != defsize) + { + /* The structure header has already been written, so */ + /* it is not possible to fall back on the internal */ + /* cell definition; the output is corrupt and the */ + /* error is fatal. */ + + TxError("Calma output error: Failed to write cell \"%s\"" + " from vendor GDS. Output is invalid!\n", + def->cd_name); + freeMagic(buffer); + FCLOSE(fi); + return 1; + } + } freeMagic(buffer); } FCLOSE(fi); diff --git a/calma/CalmaWriteZ.c b/calma/CalmaWriteZ.c index c2e9bf86..38f5a2ba 100644 --- a/calma/CalmaWriteZ.c +++ b/calma/CalmaWriteZ.c @@ -326,7 +326,7 @@ CalmaWriteZ( * to insure that each child cell is output before it is used. The * root cell is output last. */ - calmaProcessDefZ(rootDef, f, CalmaDoLibrary); + good = (calmaProcessDefZ(rootDef, f, CalmaDoLibrary) == 0) ? TRUE : FALSE; /* * Check for any cells that were instanced in the output definition @@ -345,7 +345,10 @@ CalmaWriteZ( extraDef = DBCellLookDef((char *)he->h_key.h_name); if (extraDef != NULL) - calmaProcessDefZ(extraDef, f, FALSE); + { + if (calmaProcessDefZ(extraDef, f, FALSE) != 0) + good = FALSE; + } else TxError("Error: Cell %s is not defined in the output file!\n", refname + 1); @@ -356,7 +359,7 @@ CalmaWriteZ( calmaOutRHZ(4, CALMA_ENDLIB, CALMA_NODATA, f); gzflush(f, Z_SYNC_FLUSH); gzerror(f, &nerr); - good = (nerr == 0) ? TRUE : FALSE; + if (nerr != 0) good = FALSE; /* See if any problems occurred */ if ((problems = (DBWFeedbackCount - oldCount))) @@ -928,11 +931,10 @@ calmaProcessDefZ( if (isReadOnly && hasContent) { - char *buffer, *offptr, *retfilename; + char *buffer, *retfilename; size_t defsize, numbytes; - z_off_t cellstart, cellend, structstart; + z_off_t cellstart, cellend; dlong cval; - int namelen; gzFile fi; /* Use PaOpen() so the paths searched are the same as were */ @@ -989,72 +991,109 @@ calmaProcessDefZ( cellend = (z_off_t)cval; cval = DBPropGetDouble(def, "GDS_BEGIN", &oldStyle); if (!oldStyle) - { cval = DBPropGetDouble(def, "GDS_START", NULL); - /* Write our own header and string name, to ensure */ - /* that the magic cell name and GDS name match. */ - - /* Output structure header */ - calmaOutRHZ(28, CALMA_BGNSTR, CALMA_I2, outf); - if (CalmaDateStamp != NULL) - calmaOutDateZ(*CalmaDateStamp, outf); - else - calmaOutDateZ(def->cd_timestamp, outf); - calmaOutDateZ(time((time_t *) 0), outf); - - /* Name structure the same as the magic cellname */ - calmaOutStructNameZ(CALMA_STRNAME, def, outf); - } - cellstart = (z_off_t)cval; - /* GDS_START has been defined as the start of data after the cell */ - /* structure name. However, a sanity check is wise, so move back */ - /* that far and make sure the structure name is there. */ - structstart = cellstart - (z_off_t)(strlen(def->cd_name)); - if (strlen(def->cd_name) & 0x1) structstart--; - structstart -= 2; + /* Sanity check: The structure must be long enough to */ + /* hold at least an ENDSTR record. */ - gzseek(fi, structstart, SEEK_SET); - - /* Read the structure name and check against the CellDef name */ - defsize = (size_t)(cellstart - structstart); - buffer = (char *)mallocMagic(defsize + 1); - numbytes = gzread(fi, buffer, sizeof(char) * (size_t)defsize); - if (numbytes == defsize) - { - buffer[defsize] = '\0'; - if (buffer[0] != 0x06 || buffer[1] != 0x06) - { - TxError("Calma output error: Structure name not found" - " at GDS file position %"DLONG_PREFIX"d\n", cellstart); - TxError("Calma output error: Can't write cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - else if (strcmp(&buffer[2], def->cd_name)) - { - TxError("Calma output warning: Structure definition has name" - " %s but cell definition has name %s.\n", - &buffer[2], def->cd_name); - TxError("The structure definition will be given the cell name.\n"); - } - } - else - { - TxError("Calma output error: Can't read cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - freeMagic(buffer); - - if (cellend < cellstart) /* Sanity check */ + if (cellend < cellstart + 4) { TxError("Calma output error: Bad vendor GDS file reference!\n"); isReadOnly = FALSE; } + + if (isReadOnly && !oldStyle) + { + /* GDS_START has been defined as the start of data after the */ + /* cell structure name. A sanity check is wise, so search */ + /* backward from that position for the STRNAME record header. */ + /* Note that the length of the structure name in the file */ + /* cannot be assumed to be the length of the cell name in */ + /* magic, because the cell may have been renamed after the */ + /* GDS file was read. */ + + z_off_t blockstart; + size_t blocksize; + int reclen; + char *sptr = NULL; + + blockstart = cellstart - (z_off_t)CALMARECORDMAX; + if (blockstart < 0) blockstart = (z_off_t)0; + blocksize = (size_t)(cellstart - blockstart); + + gzseek(fi, blockstart, SEEK_SET); + buffer = (char *)mallocMagic(blocksize + 1); + numbytes = gzread(fi, buffer, sizeof(char) * blocksize); + if (numbytes == blocksize) + { + buffer[blocksize] = '\0'; + + /* Scan candidate record lengths from shortest to longest. */ + /* A structure name contains only printable characters, so */ + /* the first match found is the STRNAME record header. */ + + for (reclen = 6; reclen <= (int)blocksize; reclen += 2) + { + char *tptr = buffer + blocksize - reclen; + if (((unsigned char)tptr[0] == (unsigned char)(reclen >> 8)) + && ((unsigned char)tptr[1] == (unsigned char)(reclen & 0xff)) + && (tptr[2] == CALMA_STRNAME) + && (tptr[3] == CALMA_ASCII)) + { + sptr = tptr; + break; + } + } + if (sptr == NULL) + { + TxError("Calma output error: Structure name not found" + " ending at GDS file position %"DLONG_PREFIX"d\n", + (dlong)cellstart); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + else if (strcmp(sptr + 4, def->cd_name)) + { + TxError("Calma output warning: Structure definition has name" + " %s but cell definition has name %s.\n", + sptr + 4, def->cd_name); + TxError("The structure definition will be given the cell name.\n"); + } + } + else + { + TxError("Calma output error: Can't read cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + freeMagic(buffer); + } else if (isReadOnly) + { + /* Old-style GDS_BEGIN points to the start of the structure */ + /* (BGNSTR record), which is copied verbatim including the */ + /* structure name; check that the BGNSTR record is there. */ + + char header[4]; + + gzseek(fi, cellstart, SEEK_SET); + numbytes = gzread(fi, header, sizeof(char) * 4); + if ((numbytes != 4) || (header[2] != CALMA_BGNSTR) + || (header[3] != CALMA_I2)) + { + TxError("Calma output error: Structure definition not found" + " at GDS file position %"DLONG_PREFIX"d\n", + (dlong)cellstart); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + } + + if (isReadOnly) { /* Important note: mallocMagic() is limited to size integer. */ /* This will fail on a structure larger than ~2GB. */ @@ -1062,46 +1101,74 @@ calmaProcessDefZ( defsize = (size_t)(cellend - cellstart); buffer = (char *)mallocMagic(defsize); + gzseek(fi, cellstart, SEEK_SET); numbytes = gzread(fi, buffer, sizeof(char) * (size_t)defsize); - if (numbytes == defsize) - { - /* Sanity check: buffer must end with a structure */ - /* definition end (record 0x07). */ - - if (buffer[defsize - 4] != 0x00 || - buffer[defsize - 3] != 0x04 || - buffer[defsize - 2] != 0x07 || - buffer[defsize - 1] != 0x00) - { - TxError("Calma output error: Structure end definition not found" - " at GDS file position %"DLONG_PREFIX"d\n", cellend); - TxError("Calma output error: Can't write cell from vendor GDS." - " Using magic's internal definition\n"); - isReadOnly = FALSE; - } - else - { - numbytes = gzwrite(outf, buffer, sizeof(char) * (size_t)defsize); - if (numbytes <= 0) - { - TxError("Calma output error: Can't write cell from " - "vendor GDS. Using magic's internal " - "definition\n"); - isReadOnly = FALSE; - } - } - } - else + if (numbytes != defsize) { TxError("Calma output error: Can't read cell from vendor GDS." " Using magic's internal definition\n"); /* Additional information as to why data did not match */ - TxError("Size of data requested: %"DLONG_PREFIX"d", defsize); - TxError("Length of data read: %"DLONG_PREFIX"d", numbytes); + TxError("Size of data requested: %"DLONG_PREFIX"d", (dlong)defsize); + TxError("Length of data read: %"DLONG_PREFIX"d", (dlong)numbytes); isReadOnly = FALSE; } + else if (buffer[defsize - 4] != 0x00 || + buffer[defsize - 3] != 0x04 || + buffer[defsize - 2] != 0x07 || + buffer[defsize - 1] != 0x00) + { + /* Sanity check: buffer must end with a structure */ + /* definition end (record 0x07). */ + + TxError("Calma output error: Structure end definition not found" + " at GDS file position %"DLONG_PREFIX"d\n", cellend); + TxError("Calma output error: Can't write cell from vendor GDS." + " Using magic's internal definition\n"); + isReadOnly = FALSE; + } + else + { + /* All sanity checks have passed, so the structure header, */ + /* name, and data can be written. Nothing may be written */ + /* to the output before this point, so that a failed check */ + /* above falls back on the cell definition in magic without */ + /* leaving a partial structure in the output. */ + + if (!oldStyle) + { + /* Write our own header and string name, to ensure */ + /* that the magic cell name and GDS name match. */ + + /* Output structure header */ + calmaOutRHZ(28, CALMA_BGNSTR, CALMA_I2, outf); + if (CalmaDateStamp != NULL) + calmaOutDateZ(*CalmaDateStamp, outf); + else + calmaOutDateZ(def->cd_timestamp, outf); + calmaOutDateZ(time((time_t *) 0), outf); + + /* Name structure the same as the magic cellname */ + calmaOutStructNameZ(CALMA_STRNAME, def, outf); + } + + numbytes = gzwrite(outf, buffer, sizeof(char) * (size_t)defsize); + if (numbytes != defsize) + { + /* The structure header has already been written, so */ + /* it is not possible to fall back on the internal */ + /* cell definition; the output is corrupt and the */ + /* error is fatal. */ + + TxError("Calma output error: Failed to write cell \"%s\"" + " from vendor GDS. Output is invalid!\n", + def->cd_name); + freeMagic(buffer); + gzclose(fi); + return 1; + } + } freeMagic(buffer); } gzclose(fi); @@ -1318,7 +1385,7 @@ calmaOutFuncZ( numports++; } } - if (newll != NULL) + if (ll != NULL) { /* Turn linked list into an array, then run qsort on it */ /* to sort by port number. */ diff --git a/calma/calmaInt.h b/calma/calmaInt.h index 7acde49c..50965665 100644 --- a/calma/calmaInt.h +++ b/calma/calmaInt.h @@ -131,6 +131,9 @@ typedef struct /* Length of record header */ #define CALMAHEADERLENGTH 4 +/* Largest possible record (limited by the 16-bit record length field) */ +#define CALMARECORDMAX 65535 + /* Label types * The intention is all the values can be stored/converted with unsigned char type, * C23 allows us to be explicit with the type but C99 does not, so a comment for now.