[Bug 192839] Duplicate entries in an mtree file cause nmtree to coredump

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192839

--- Comment #7 from Felix Dietrich <felix.dietrich+freebsd-bugtracker@sperrhaken.name> ---
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.