Hi,
as reported by Federico Schwindt, when running with malloc.conf -> S
on amd64, "man -a x" crashes if the list of directory patterns to
search for manuals (for example constructed from _default and _subdir
in man.conf(5)) contains at least one entry expanding to no existing
directories. The problem is reproducible on i386.
This happens because the manual() function in man.c first free(3)s
a TAILQ entry, then uses the freed entry to advance to the next entry.
Fix that by correcting the order of operations in the loop:
1) expand the entry
2) insert the results before it
3) get a pointer to the next entry
4) remove and free the processed entry
Instead of doing 3) and 4) in the reverse order.
While here, make the code more intelligible by always using e_tag
to iterate the list and always using ep to insert and remove entries.
Tested on i386 and sparc64.
Thanks to fgsch@ for reporting the bug, to otto@ for providing S,
and no cookies for me for not using S when i introduced the bug
in rev. 1.43 of man.c, July 2011.
OK?
Ingo
Index: man.c
===================================================================
RCS file: /cvs/src/usr.bin/man/man.c,v
retrieving revision 1.43
diff -u -p -r1.43 man.c
--- man.c 7 Jul 2011 04:24:35 -0000 1.43
+++ man.c 5 Jan 2012 02:05:39 -0000
@@ -2,7 +2,7 @@
/* $NetBSD: man.c,v 1.7 1995/09/28 06:05:34 tls Exp $ */
/*
- * Copyright (c) 2010, 2011 Ingo Schwarze <schwa...@openbsd.org>
+ * Copyright (c) 2010, 2011, 2012 Ingo Schwarze <schwa...@openbsd.org>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -443,7 +443,7 @@ manual(char *page, TAG *tag, glob_t *pg)
/* Expand the search path. */
if (f_all != f_where) {
e_tag = tag == NULL ? NULL : TAILQ_FIRST(&tag->list);
- for (; e_tag != NULL; e_tag = TAILQ_NEXT(e_tag, q)) {
+ while (e_tag != NULL) {
if (glob(e_tag->s, GLOB_BRACE | GLOB_NOSORT,
NULL, pg)) {
/* No GLOB_NOMATCH here due to {arch,}. */
@@ -451,17 +451,18 @@ manual(char *page, TAG *tag, glob_t *pg)
(void)cleanup(0);
exit(1);
}
- ep = e_tag;
for (cnt = 0; cnt < pg->gl_pathc; cnt++) {
- if ((e_tag = malloc(sizeof(ENTRY))) == NULL ||
- (e_tag->s = strdup(pg->gl_pathv[cnt])) ==
+ if ((ep = malloc(sizeof(ENTRY))) == NULL ||
+ (ep->s = strdup(pg->gl_pathv[cnt])) ==
NULL) {
warn(NULL);
(void)cleanup(0);
exit(1);
}
- TAILQ_INSERT_BEFORE(ep, e_tag, q);
+ TAILQ_INSERT_BEFORE(e_tag, ep, q);
}
+ ep = e_tag;
+ e_tag = TAILQ_NEXT(e_tag, q);
free(ep->s);
TAILQ_REMOVE(&tag->list, ep, q);
free(ep);
