Bug 192839
| Summary: | Duplicate entries in an mtree file cause nmtree to coredump | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Enji Cooper <ngie> |
| Component: | bin | Assignee: | Jose Luis Duran <jlduran> |
| Status: | In Progress --- | ||
| Severity: | Affects Some People | CC: | bdrewery, brooks, emaste, felix.dietrich+freebsd-bugtracker, jlduran, lwhsu, ngie, pat |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
|
Description
Enji Cooper
2014-08-19 20:49:57 UTC
An even simpler repro is:
% /bin/sh
% nmtree -dEU <<EOF
.
duplicate_entry
..
duplicate_entry
..
..
EOF
Bus error: 10 (core dumped)
%
*sigh*. This impacts makefs too, making makefs -D useless. Another case that repros this is as follows (not sure if this mtree file is valid though):
.
a
..
..
.
a
..
..
Handing a number of makefs, mtree, and msdosfs bugs in my queue over to emaste@. I do not expect to be able to look at mtree bugs in the near term. I am unable to reproduce the core dump with the provided examples: The example of comment #1 yields (the “-E” to exclude a tag should presumably have been a “-e” to not complain about files not in the specification) : nmtree: no parent node nmtree: failed at line 6 of the specification For the example in comment #3 only the line number in the output differs: nmtree: no parent node nmtree: failed at line 4 of the specification A slight modification to the example found in comment #1 (specifying the default type to be “dir”) /set type=dir . duplicate_entry .. duplicate_entry .. .. yields, on an empty root directory, the following: ./dup missing (directory not created: user not specified) The problem that brought me here is a segmentation fault resulting from feeding the following example to nmtree:
/set type=dir
.
dup
..
dup
child_entry type=file
Line numbers in the following paragraph are according to FreeBSD 12.4-RELEASE-p2 (git hash 149768b65d619833331bcf0c2fb121b20643f2f1).
The segmentation fault is caused by using the memory pointed to by centry (“last = cenry” in the “spec” function [spec.c:252]) after it has been freed in replacenode (“free(new)” [spec.c:536]). (A comment in the addchild function [spec.c:775] indicates that at least addchild is aware that replacenode will free the centry.) The memory gets reused for the next centry (“centry = calloc(…” [spec.c:207]) and assigned the global defaults (“*centry = ginfo” [spec.c:209]). The parent member of the ginfo record is NULL [spec.c:122], and, as a consequence of the faulty memory reuse, addchild gets passed NULL (“addchild(last->parent, centry)” [spec.c:260]) for its pathparent parameter. This results in a segmentation fault early in addchild because of a NULL dereference (“cur = pathparent->child” [spec.c:734]). Also note that last->type gets overridden and set to F_FILE [spec.c:215], and, therefore, not the branch “last->type == F_DIR && !(last->flags & F_DONE)” [spec.c:245] is taken, as one would expect by correct operation, but the final else branch “new relative child in parent dir” [spec.c:253].
I have not reasoned much about solutions. It might be possible for a quick and dirty fix to dispense with the “free(new)” call in replacenode [spec.c:536] at the cost of leaking memory and making the code even harder to reason about. Another solutions could make replacenode actually replace the node (removing “cur” from the data structure and emplacing “new” in its stead) instead of overwriting the members of the existing “cur” node – but I have not tried to understand and follow the assumptions addchild makes of replacenodeʼs operations.
The bug has been reported upstream, with a very naive workaround that can be used in the meantime a real fix is issued. https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=58887 |