Bug 203531 - makefs causes ISO 9660 flaws in FreeBSD-11.0-CURRENT-amd64-*-disc1.iso
Summary: makefs causes ISO 9660 flaws in FreeBSD-11.0-CURRENT-amd64-*-disc1.iso
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-04 10:25 UTC by scdbackup
Modified: 2018-05-28 20:35 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description scdbackup 2015-10-04 10:25:29 UTC
Hi,

i see some ISO 9660 and/or Rock Ridge problems in

  FreeBSD-11.0-CURRENT-amd64-20151001-r288459-disc1.iso

from
  http://ftp.freebsd.org/%2Fpub/FreeBSD/snapshots/amd64/amd64/ISO-IMAGES/11.0/

Garrett Cooper told me on freebsd-hackers@freebsd.org that
this ISO was made by 
  https://svnweb.freebsd.org/base/head/release/amd64/mkisoimages.sh?view=log
with backend makefs. I assume that the source at

  https://svnweb.freebsd.org/base/head/usr.sbin/makefs/

is the one in effect.
He advised me to file a PR with "CC re@".

I deem the problems closely related enough for sharing one PR.
Please give me a note if i shall re-submit as separate PRs.

Overview:

1: Uninitialized malloc memory in timestamp of root directory.

2: Timestamp of "/bin" differs from timestamp of "/bin/."

3: Rock Ridge timestamp entry TF shows atime as Creation Time
   and ctime as Access Time.

4: Directories with all uppercase names get shown as lowercase
   on Linux.

===================================================================
Problem 1:
Uninitialized malloc memory in timestamp of root directory.

The root directory entry and also "/." show as "Recording Date
and Time" the 7-byte string {165, ..., 165}, which except for the
first byte violates ECMA-119 9.1.5. It restricts bytes 1 to 5 to
reasonable values for month, day_of_month, hour, minute, second.
Byte 6 shall be in the range of -48 to +52, whereas 165 as
signed 8-byte value is -91.

--------------- Source analysis:

  usr.sbin/makefs/cd9660.c : cd9660_populate_iso_dir_record()

does not set a default value for

  record->date

Function cd9660_translate_node_common() does after the populate
call:

        /* If we want to use the current date and time */
        time(&tim);
        cd9660_time_915(newnode->isoDirRecord->date, tim);

The function 

  usr.bin/makefs/cd9660/cd9660_conversion.c : cd9660_time_915()

looks like an implementation of ECMA-119 9.1.5, the appropriate
format for record->date.

Function cd9660_setup_root_node() does not make such a call for

  diskStructure.rootNode->isoDirRecord->date

--------------- Remedy proposal:

Move the default timestamp code from cd9660_translate_node_common()
to cd9660_populate_iso_dir_record().


===================================================================
Problem 2:
Timestamp of "/bin" differs from timestamp of "/bin/."

The Rock Ridge equipment of directory records of directories in
their parent directory differs from the equipment of "." records
in their own directory.

E.g. "/bin" differs from "/bin/." not only by name.
"/bin" has Rock Ridge entry TF, which gives timestamps.
  (At 2048-block 842 + offset 584 bytes)
"/bin/." has no TF.
  (At 2048-block 844 + offset 0)
The content of TF's Creation Time
  {115  10, 1, 21, 36, 58, 0}
differs from the content of the ECMA-119 record fields
  {115, 10, 1, 21, 38, 24, 0}

--------------- Source analysis:

  usr.sbin/makefs/cd9660/iso9660_rrip.c : cd9660_rrip_initialize_node()

has special handling for CD9660_TYPE_DOT and CD9660_TYPE_DOTDOT,
which only appends PX entries.
Other nodes get treated by cd9660_rrip_initialize_inode(), which
appends TF.

--------------- Remedy proposal:

Add adapted copies of the TF producer from cd9660_rrip_initialize_inode():

        /* TF - timestamp */
        attr = cd9660node_susp_create_node(SUSP_TYPE_RRIP,
                SUSP_ENTRY_RRIP_TF, "TF", SUSP_LOC_ENTRY);
        cd9660node_rrip_tf(attr, node->node);
        TAILQ_INSERT_TAIL(&node->head, attr, rr_ll);

to the cases in cd9660_rrip_initialize_node():

        if (node->type & CD9660_TYPE_DOT) {
                ...
                if (parent != NULL && parent->node != NULL &&
                    parent->node->inode != NULL) {
                        /* PX - POSIX attributes */
                        current = cd9660node_susp_create_node(SUSP_TYPE_RRIP,
                                SUSP_ENTRY_RRIP_PX, "PX", SUSP_LOC_ENTRY);
                        cd9660node_rrip_px(current, parent->node);
                        TAILQ_INSERT_TAIL(&node->head, current, rr_ll);

                        /* TF - timestamp */
                        current = cd9660node_susp_create_node(SUSP_TYPE_RRIP,
                                SUSP_ENTRY_RRIP_TF, "TF", SUSP_LOC_ENTRY);
                        cd9660node_rrip_tf(attr, parent->node);
                        TAILQ_INSERT_TAIL(&node->head, current, rr_ll);

                }
         } else if (node->type & CD9660_TYPE_DOTDOT) {
                if (grandparent != NULL && grandparent->node != NULL &&
                    grandparent->node->inode != NULL) {
                        /* PX - POSIX attributes */
                        ...
                        TAILQ_INSERT_TAIL(&node->head, current, rr_ll);

                        /* TF - timestamp */
                        current = cd9660node_susp_create_node(SUSP_TYPE_RRIP,
                                SUSP_ENTRY_RRIP_TF, "TF", SUSP_LOC_ENTRY);
                        cd9660node_rrip_tf(attr, grandparent->node));
                        TAILQ_INSERT_TAIL(&node->head, current, rr_ll);

                 }
                 /* Handle PL */


===================================================================
Problem 3:
Rock Ridge timestamp entry TF shows atime as Creation Time and
ctime as Access Time.

The Rock Ridge TF entries indicate Creation Time rather than
Last Attribute Change Time.
RRIP-1.12 says:
  "If recorded, CREATION, Creation Time, has the same meaning as in
   ISO 9660:9.5.4."
  "If recorded, ATTRIBUTES, Last Attribute Change Time, shall be
   used for the st_ctime field of POSIX:5.6.1."
ECMA-119 (aka ISO 9660):
  "9.5.4 File Creation Date and Time (BP 11 to 27)
   This field shall specify the date and the time of the day at
   which the information in the file was created."
So for recording ctime, the FreeBSD ISO seems to use the wrong TF
field. (I write "seems" because it is actually worse than that.)

--------------- Source analysis:

TF is composed in

  usr.sbin/makefs/cd9660/iso9660_rrip.c : cd9660node_rrip_tf()

The TF Flags value is composed as

      p->attr.rr_entry.TF.flags[0] = TF_MODIFY | TF_ACCESS | TF_ATTRIBUTES;

which on the first glimpse looks ok. But

  usr.sbin/makefs/cd9660/iso9660_rrip.h

has wrong values for the macros

        #define TF_CREATION 0x00
        #define TF_MODIFY 0x01
        #define TF_ACCESS 0x02
        #define TF_ATTRIBUTES 0x04

According to RRIP-1.12 Table 5 the defines should be

        #define TF_CREATION 0x01
        #define TF_MODIFY 0x02
        #define TF_ACCESS 0x04
        #define TF_ATTRIBUTES 0x08

The timestamps atime, mtime, ctime are then written in wrong order
so that at least mtime ends up where a mounter would expect it
with the wrong flags value 0x07. ctime and atime are wrongly
positioned even for a very tolerant reader like the Linux kernel.

RRIP-1.12 prescribes to write the values in the order of the
flags bits. I.e.: creation_time, mtime, atime, ctime, ...

--------------- Remedy proposal:

In  usr.sbin/makefs/cd9660/iso9660_rrip.h define

        #define TF_CREATION 0x01
        #define TF_MODIFY 0x02
        #define TF_ACCESS 0x04
        #define TF_ATTRIBUTES 0x08

and in usr.sbin/makefs/cd9660/iso9660_rrip.c rearrange the three
time stamp values to

        cd9660_time_915(p->attr.rr_entry.TF.timestamp,
                        _node->inode->st.st_mtime);
        ...
        cd9660_time_915(p->attr.rr_entry.TF.timestamp,
                        _node->inode->st.st_atime);
        ...
        cd9660_time_915(p->attr.rr_entry.TF.timestamp,
                        _node->inode->st.st_ctime);
        ...


===================================================================
Problem 4:
Directories with all uppercase names get shown as lowercase
on Linux.

Some files have Rock Ridge NM fields, some don't.
The NM field records the case sensitive long name of the file.
Having none makes the file name prone to mapping when it gets
shown by reader software. Typical mappings are:
- Removal of trailing ".;1" or ";1".
- Presentation as lowercase characters.
Missing are the NM fields of the directories in
  /usr/share/i18n/csmapper
    (At 2048-block 323995)
  /usr/share/i18n/esdb
    (At 2048-block 338174)
and of the directory "C" in
  /usr/share/nls
    (At 2048-block 353273)
The regular files in the same directories do have NM.

--------------- Source analysis:

  usr.sbin/makefs/cd9660/iso9660_rrip.c : cd9660_rrip_initialize_node()

has the following conceptual mistake:

       /*
        * Not every node needs a NM set - only if the name is
        * actually different. IE: If a file is TEST -> TEST,
        * no NM. test -> TEST, need a NM
        *
        * The rr_moved_dir needs to be assigned a NM record as well.
        */

This explains why only directories are affected. Names of regular
files get appended a dot ".", if there is none in the name, plus
text ";1" as "Version Number".

As mentioned above, the lack of NM invites name mapping.
E.g. the kernel of Linux by default maps ECMA-119 names to lowercase.
FreeBSD 8.0 (antique) and NetBSD 6.99 (of last year) show uppercase,
although man mount_cd9660 of NetBSD indicates otherwise in its text
about "-o gens".

--------------- Remedy proposal:

Unconditionally call

               cd9660_rrip_NM(node);

for all nodes which are not CD9660_TYPE_DOT or CD9660_TYPE_DOTDOT.


===================================================================

Have a nice day :)

Thomas
Comment 1 scdbackup 2015-10-20 07:01:26 UTC
All four problems are present in NetBSD's makefs, too.

===================================================================
Problem 1:
Uninitialized malloc memory in timestamp of root directory.

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660.c?annotate=1.49

The function cd9660_populate_iso_dir_record (line 733)
looks exactly like in FreeBSD.

cd9660_setup_root_node() does not set a date, whereas
cd9660_translate_node_common() does.

===================================================================
Problem 2:
Timestamp of "/bin" differs from timestamp of "/bin/."

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/iso9660_rrip.c?annotate=1.11.2.2

380:        if (node->type & CD9660_TYPE_DOT) {

                    ... no TF ...

397:        } else if (node->type & CD9660_TYPE_DOTDOT) {

                    ... no TF ...

413:        } else {
414:                cd9660_rrip_initialize_inode(node);

===================================================================
Problem 3:
Rock Ridge timestamp entry TF shows atime as Creation Time and
ctime as Access Time.

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/iso9660_rrip.h?annotate=1.5.8.1

51: #define         TF_CREATION       0x00
52: #define         TF_MODIFY         0x01
53: #define         TF_ACCESS         0x02
54: #define         TF_ATTRIBUTES     0x04

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/iso9660_rrip.c?annotate=1.11.2.2

690:        p->attr.rr_entry.TF.flags[0] = TF_MODIFY | TF_ACCESS | TF_ATTRIBUTES;
...
700:                _node->inode->st.st_atime);
...
704:                _node->inode->st.st_mtime);
...
708:                _node->inode->st.st_ctime);


===================================================================
Problem 4:
Directories with all uppercase names get shown as lowercase
on Linux.

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/iso9660_rrip.c?annotate=1.11.2.2

417:                 * Not every node needs a NM set - only if the name is
418:                 * actually different. IE: If a file is TEST -> TEST,
419:                 * no NM. test -> TEST, need a NM

===================================================================
Comment 2 Enji Cooper freebsd_committer 2017-11-05 20:59:20 UTC
Handing a number of makefs, mtree, and msdosfs bugs in my queue over to emaste@.
Comment 3 Ed Maste freebsd_committer 2018-05-28 20:35:53 UTC
Reset assignee - I am not currently looking at this PR.