Date: Thu, 3 Jan 2008 16:28:46 GMT From: Doug Moore <dougm@ironport.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/119307: TRASHIT macro blasts list header if REMOVE invoked with header ref Message-ID: <200801031628.m03GSkU2052315@freefall.freebsd.org> Resent-Message-ID: <200801031630.m03GU305052531@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 119307
>Category: kern
>Synopsis: TRASHIT macro blasts list header if REMOVE invoked with header ref
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Jan 03 16:30:03 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator: Doug Moore <dougm@ironport.com>
>Release: FreeBSD 7.0-CURRENT i386
>Organization:
>Environment:
System: FreeBSD adsl-216-63-78-19.dsl.hstntx.swbell.net 7.0-CURRENT FreeBSD 7.0-CURRENT #5: Sat Sep 15 00:42:55 CDT 2007 root@adsl-216-63-78-19.dsl.hstntx.swbell.net:/usr/src/sys/i386/compile/ATHEROS i386
>Description:
A line like
TAILQ_REMOVE(&a->tailq, a->tailq.tqh_first, next);
will corrupt the tailq in question if QUEUE_MACRO_DEBUG is on,
since after the first tailq item is removed, it is the *new*
first tailq item that gets its prev and next pointers trashed.
With QUEUE_MACRO_DEBUG off, it removes the first tailq as
expected. Therefore, turning on QUEUE_MACRO_DEBUG corrupts
the tailq.
>How-To-Repeat:
Write a self-referential invocation of a removal from tailq,
list, etc, and add a couple of items to the list before
invoking that removal.
>Fix:
Use typeof to bind the value of the to-be-removed element to a
name, and use that name in the pointer trashing and other
debugging code. If typeof is forbidden, then it would seem to
require adding type arguments to several macros, a bad thing.
--- queue.h.diff begins here ---
--- queue.h 2008-01-02 22:53:54.000000000 -0600
+++ queue.h.new 2008-01-02 23:02:28.000000000 -0600
@@ -188,6 +188,7 @@ struct { \
#define SLIST_NEXT(elm, field) ((elm)->field.sle_next)
#define SLIST_REMOVE(head, elm, type, field) do { \
+ typeof (elm) oldelm = (elm); \
if (SLIST_FIRST((head)) == (elm)) { \
SLIST_REMOVE_HEAD((head), field); \
} \
@@ -198,7 +199,7 @@ struct { \
SLIST_NEXT(curelm, field) = \
SLIST_NEXT(SLIST_NEXT(curelm, field), field); \
} \
- TRASHIT((elm)->field.sle_next); \
+ TRASHIT(oldelm->field.sle_next); \
} while (0)
#define SLIST_REMOVE_HEAD(head, field) do { \
@@ -280,6 +281,7 @@ struct { \
#define STAILQ_NEXT(elm, field) ((elm)->field.stqe_next)
#define STAILQ_REMOVE(head, elm, type, field) do { \
+ typeof (elm) oldelm = (elm); \
if (STAILQ_FIRST((head)) == (elm)) { \
STAILQ_REMOVE_HEAD((head), field); \
} \
@@ -291,7 +293,7 @@ struct { \
STAILQ_NEXT(STAILQ_NEXT(curelm, field), field)) == NULL)\
(head)->stqh_last = &STAILQ_NEXT((curelm), field);\
} \
- TRASHIT((elm)->field.stqe_next); \
+ TRASHIT(oldelm->field.stqe_next); \
} while (0)
#define STAILQ_REMOVE_HEAD(head, field) do { \
@@ -392,14 +394,15 @@ struct { \
#define LIST_NEXT(elm, field) ((elm)->field.le_next)
#define LIST_REMOVE(elm, field) do { \
+ typeof (elm) oldelm = (elm); \
QMD_LIST_CHECK_NEXT(elm, field); \
QMD_LIST_CHECK_PREV(elm, field); \
if (LIST_NEXT((elm), field) != NULL) \
LIST_NEXT((elm), field)->field.le_prev = \
(elm)->field.le_prev; \
*(elm)->field.le_prev = LIST_NEXT((elm), field); \
- TRASHIT((elm)->field.le_next); \
- TRASHIT((elm)->field.le_prev); \
+ TRASHIT(oldelm->field.le_next); \
+ TRASHIT(oldelm->field.le_prev); \
} while (0)
/*
@@ -554,6 +557,7 @@ struct { \
(*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
#define TAILQ_REMOVE(head, elm, field) do { \
+ typeof (elm) oldelm = (elm); \
QMD_TAILQ_CHECK_NEXT(elm, field); \
QMD_TAILQ_CHECK_PREV(elm, field); \
if ((TAILQ_NEXT((elm), field)) != NULL) \
@@ -564,9 +568,9 @@ struct { \
QMD_TRACE_HEAD(head); \
} \
*(elm)->field.tqe_prev = TAILQ_NEXT((elm), field); \
- TRASHIT((elm)->field.tqe_next); \
- TRASHIT((elm)->field.tqe_prev); \
- QMD_TRACE_ELEM(&(elm)->field); \
+ TRASHIT(oldelm->field.tqe_next); \
+ TRASHIT(oldelm->field.tqe_prev); \
+ QMD_TRACE_ELEM(&oldelm->field); \
} while (0)
--- queue.h.diff ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200801031628.m03GSkU2052315>