Summary: | makefs: Coverity CID 976312: SIGSEGV with option -l 3 | ||
---|---|---|---|
Product: | Base System | Reporter: | scdbackup |
Component: | bin | Assignee: | Enji Cooper <ngie> |
Status: | Closed FIXED | ||
Severity: | Affects Some People | CC: | emaste, ngie |
Priority: | --- | Flags: | ngie:
mfc-stable10+
ngie: mfc-stable9+ ngie: mfc-stable8- |
Version: | CURRENT | ||
Hardware: | Any | ||
OS: | Any |
Description
scdbackup
2015-10-08 18:41:23 UTC
Also present in NetBSD. http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660.c?annotate=1.49 280: OPT_NUM('l', "isolevel", isoLevel, 281: 1, 3, "ISO Level"), 879: if (!(S_ISDIR(node->type))) 880: newnode->fileDataLength = node->inode->st.st_size; 1772: else if (diskStructure->isoLevel == 2) 1773: conversion_function = &cd9660_level2_convert_filename; 1774: return (*conversion_function)(diskStructure, oldname, newname, is_file); A commit references this bug: Author: ngie Date: Mon Nov 2 08:43:15 UTC 2015 New revision: 290264 URL: https://svnweb.freebsd.org/changeset/base/290264 Log: Limit isoLevel to 1 and 2 to avoid segfaulting when isoLevel is set to 3 by dereferencing a NULL function pointer Add some asserts to ensure that isolevel is always either 1 or 2. PR: 203645 Reported by: Thomas Schmitt <scdbackup@gmx.net> MFC after: 1 week Sponsored by: EMC / Isilon Storage Division Changes: head/usr.sbin/makefs/cd9660.c A commit references this bug: Author: ngie Date: Mon Nov 2 09:16:51 UTC 2015 New revision: 290265 URL: https://svnweb.freebsd.org/changeset/base/290265 Log: Add testcases for -t cd9660 -o isolevel=[1-3] -- -o isolevel=1 currently fails because of path comparison issues, so mark it as an expected failure. -- -o isolevel=3 is not implemented, so expect it to fail as an out of bounds value [*]. PR: 203645 MFC after: 1 week X-MFC with: r290264 Sponsored by: EMC / Isilon Storage Division Changes: head/usr.sbin/makefs/tests/makefs_cd9660_tests.sh The patch does not care for oversized data files, which currently get misrepresented silently. Such a data file should rather be excluded or taken as reason for abort. A commit references this bug: Author: ngie Date: Mon Nov 9 09:20:01 UTC 2015 New revision: 290594 URL: https://svnweb.freebsd.org/changeset/base/290594 Log: MFC r290265,r290267,r290270: r290265: Add testcases for -t cd9660 -o isolevel=[1-3] -- -o isolevel=1 currently fails because of path comparison issues, so mark it as an expected failure. -- -o isolevel=3 is not implemented, so expect it to fail as an out of bounds value [*]. PR: 203645 Sponsored by: EMC / Isilon Storage Division r290267: Clean up mtree keyword support a slight bit and add a few more default keywords - Parameterize the mtree keywords as $DEFAULT_MTREE_KEYWORDS - Test with the extra mtree keywords, `mode,gid,uid`. - Add a note about mtrees with time support not working with makefs right now Sponsored by: EMC / Isilon Storage Division r290270: Add testcases for -t ffs -o version=[12] Verify the filesystem type using dumpfs. Add preliminary support for NetBSD (needs to be validated) Sponsored by: EMC / Isilon Storage Division Changes: _U stable/10/ stable/10/usr.sbin/makefs/tests/makefs_cd9660_tests.sh stable/10/usr.sbin/makefs/tests/makefs_ffs_tests.sh stable/10/usr.sbin/makefs/tests/makefs_tests_common.sh A commit references this bug: Author: ngie Date: Mon Nov 9 09:22:12 UTC 2015 New revision: 290595 URL: https://svnweb.freebsd.org/changeset/base/290595 Log: MFC r290264: Limit isoLevel to 1 and 2 to avoid segfaulting when isoLevel is set to 3 by dereferencing a NULL function pointer Add some asserts to ensure that isolevel is always either 1 or 2. PR: 203645 Reported by: Thomas Schmitt <scdbackup@gmx.net> Sponsored by: EMC / Isilon Storage Division Changes: _U stable/10/ stable/10/usr.sbin/makefs/cd9660.c A commit references this bug: Author: ngie Date: Mon Nov 9 09:23:13 UTC 2015 New revision: 290596 URL: https://svnweb.freebsd.org/changeset/base/290596 Log: MFstable/10 r290595: MFC r290264: Limit isoLevel to 1 and 2 to avoid segfaulting when isoLevel is set to 3 by dereferencing a NULL function pointer Add some asserts to ensure that isolevel is always either 1 or 2. PR: 203645 Reported by: Thomas Schmitt <scdbackup@gmx.net> Sponsored by: EMC / Isilon Storage Division Changes: _U stable/9/ _U stable/9/usr.sbin/ _U stable/9/usr.sbin/makefs/ stable/9/usr.sbin/makefs/cd9660.c (In reply to scdbackup from comment #4) > The patch does not care for oversized data files, > which currently get misrepresented silently. > Such a data file should rather be excluded or > taken as reason for abort. ISO-9660 level 1/2 both need to be fixed and ISO-9660 level 3 needs to be completely implemented. Feel free to post follow up bugs. I just wanted to make sure the segfault was corrected as implementing level 3 seemed like a larger task to perform for this bug. I agree that it is most important to put a cap on that SIGSEGV. But implementing multi-extent files (which effectively constitute ISO 9660 level 3) would be a substantial change. Further it will not be enough to enhance makefs alone, because the readonly filesystem implementation in the FreeBSD kernel is unable to properly represent such files. So for now - and only to complete the safety cap - makefs should refuse to record data files which must be represented as multi-extent files. I.e. files with more than 4 GiB -1 bytes of content. This test is needed anyways to make sure that chosen level 1 and 2 do not exceed the limits which are specified in ECMA-119 chapter 10 "Levels of interchange": "10.1 Level 1. - each file shall consist of only one File Section; [...] 10.2 Level 2 - each file shall consist of only one File Section; [...] " File Section is what other filesystems name "Extent". The byte counter of a File Section is an unsigned 32 bit number (written doubly in both endiannesses, as usual in ISO 9660). So if the --isolevel option is given, then user may expect to be protected from shooting the own foot by oversized data files. (In reply to scdbackup from comment #9) Closing bug. Will talk with emaste about implementing the enhancements discussed in comment # 9 and will CC you on the bug. Please don't reopen this bug. |