Skip to content

Commit

Permalink
makefs: Fix cd9660 filename buffer maximum length
Browse files Browse the repository at this point in the history
The maximum length is 30 characters for name and extension, two
separators (. and ;) and 5 characters for file version from 1 to 32767,
which is 37 characters.  Add one for the null term as we treat these
buffers as C strings.

This is not an issue in practice, as the file version is always 1 in
makefs.

While here, drop `_WITH_PADDING` from the macro name and update the
previously-unused ISO_FILENAME_MAXLENGTH for the corrected length.
A 0x00 padding byte is used by ISO9660 when needed for alignment, which
can be the null byte at the end of the string.

Use sizeof where appropriate.

Reviewed by:	kevans
Sponsored by:	The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D48193
  • Loading branch information
emaste committed Dec 26, 2024
1 parent ca7e12f commit 2e09cef
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
16 changes: 7 additions & 9 deletions usr.sbin/makefs/cd9660.c
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ cd9660_makefs(const char *image, const char *dir, fsnode *root,
real_root->isoDirRecord = emalloc(sizeof(*real_root->isoDirRecord));
/* Leave filename blank for root */
memset(real_root->isoDirRecord->name, 0,
ISO_FILENAME_MAXLENGTH_WITH_PADDING);
sizeof(real_root->isoDirRecord->name));

real_root->level = 0;
diskStructure->rootNode = real_root;
Expand Down Expand Up @@ -796,10 +796,10 @@ static int
cd9660_translate_node_common(iso9660_disk *diskStructure, cd9660node *newnode)
{
u_char flag;
char temp[ISO_FILENAME_MAXLENGTH_WITH_PADDING];
char temp[ISO_FILENAME_MAXLENGTH];

/* Now populate the isoDirRecord structure */
memset(temp, 0, ISO_FILENAME_MAXLENGTH_WITH_PADDING);
memset(temp, 0, sizeof(temp));

(void)cd9660_convert_filename(diskStructure, newnode->node->name,
temp, sizeof(temp), !(S_ISDIR(newnode->node->type)));
Expand Down Expand Up @@ -1044,7 +1044,7 @@ cd9660_rename_filename(iso9660_disk *diskStructure, cd9660node *iter, int num,
else
maxlength = ISO_FILENAME_MAXLENGTH_BEFORE_VERSION;

tmp = emalloc(ISO_FILENAME_MAXLENGTH_WITH_PADDING);
tmp = emalloc(ISO_FILENAME_MAXLENGTH);

while (i < num && iter) {
powers = 1;
Expand Down Expand Up @@ -1105,8 +1105,7 @@ cd9660_rename_filename(iso9660_disk *diskStructure, cd9660node *iter, int num,
while (digits > 0) {
digit = (int)(temp / powers);
temp = temp - digit * powers;
snprintf(&tmp[numbts],
ISO_FILENAME_MAXLENGTH_WITH_PADDING - numbts,
snprintf(&tmp[numbts], ISO_FILENAME_MAXLENGTH - numbts,
"%d", digit);
digits--;
numbts++;
Expand Down Expand Up @@ -1153,8 +1152,7 @@ cd9660_copy_filenames(iso9660_disk *diskStructure, cd9660node *node)

TAILQ_FOREACH(cn, &node->cn_children, cn_next_child) {
cd9660_copy_filenames(diskStructure, cn);
memcpy(cn->o_name, cn->isoDirRecord->name,
ISO_FILENAME_MAXLENGTH_WITH_PADDING);
memcpy(cn->o_name, cn->isoDirRecord->name, sizeof(cn->o_name));
}
}

Expand Down Expand Up @@ -1277,7 +1275,7 @@ cd9660_rrip_move_directory(iso9660_disk *diskStructure, cd9660node *dir)
/* TODO: Inherit permissions / ownership (basically the entire inode) */

/* Set the new name */
memset(dir->isoDirRecord->name, 0, ISO_FILENAME_MAXLENGTH_WITH_PADDING);
memset(dir->isoDirRecord->name, 0, sizeof(dir->isoDirRecord->name));
strncpy(dir->isoDirRecord->name, newname, 8);
dir->isoDirRecord->length[0] = 34 + 8;
dir->isoDirRecord->name_len[0] = 8;
Expand Down
9 changes: 4 additions & 5 deletions usr.sbin/makefs/cd9660.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@

/*30 for name and extension, as well as version number and padding bit*/
#define ISO_FILENAME_MAXLENGTH_BEFORE_VERSION 30
#define ISO_FILENAME_MAXLENGTH 36
#define ISO_FILENAME_MAXLENGTH_WITH_PADDING 37
#define ISO_FILENAME_MAXLENGTH 38

#define ISO_FLAG_CLEAR 0x00
#define ISO_FLAG_HIDDEN 0x01
Expand Down Expand Up @@ -118,7 +117,7 @@ typedef struct _iso_directory_record_cd9660 {
u_char interleave [ISODCL (28, 28)]; /* 711 */
u_char volume_sequence_number [ISODCL (29, 32)]; /* 723 */
u_char name_len [ISODCL (33, 33)]; /* 711 */
char name [ISO_FILENAME_MAXLENGTH_WITH_PADDING];
char name [ISO_FILENAME_MAXLENGTH];
} iso_directory_record_cd9660;

/* TODO: Lots of optimization of this structure */
Expand Down Expand Up @@ -154,7 +153,7 @@ typedef struct _cd9660node {
int fileRecordSize;/*copy of a variable, int for quicker calculations*/

/* Old name, used for renaming - needs to be optimized but low priority */
char o_name [ISO_FILENAME_MAXLENGTH_WITH_PADDING];
char o_name [ISO_FILENAME_MAXLENGTH];

/***** SPACE RESERVED FOR EXTENSIONS *****/
/* For memory efficiency's sake - we should move this to a separate struct
Expand Down Expand Up @@ -194,7 +193,7 @@ typedef struct _path_table_entry
u_char extended_attribute_length[ISODCL (2, 2)];
u_char first_sector[ISODCL (3, 6)];
u_char parent_number[ISODCL (7, 8)];
char name[ISO_FILENAME_MAXLENGTH_WITH_PADDING];
char name[ISO_FILENAME_MAXLENGTH];
} path_table_entry;

typedef struct _volume_descriptor
Expand Down

0 comments on commit 2e09cef

Please sign in to comment.