Date: Fri, 24 Mar 2000 18:06:08 +0000 (GMT) From: iedowse@maths.tcd.ie To: FreeBSD-gnats-submit@freebsd.org Subject: kern/17583: NETATALK code can corrupt mbuf free lists Message-ID: <200003241806.aa98076@walton.maths.tcd.ie>
next in thread | raw e-mail | index | archive | help
>Number: 17583
>Category: kern
>Synopsis: NETATALK code can corrupt mbuf free lists
>Confidential: no
>Severity: serious
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Fri Mar 24 10:10:01 PST 2000
>Closed-Date:
>Last-Modified:
>Originator: Ian Dowse
>Release: FreeBSD 3.4-STABLE i386
>Organization:
School of Mathematics
Trinity College, Dublin
>Environment:
FreeBSD -current, but problem exists in 4.0-STABLE and 3.x
>Description:
In certain circumstances, the NETATALK code can cause an mbuf
chain to be freed multiple times.
The code uses a 'struct aarptab' to store information about
appletalk addresses, which includes an mbuf pointer 'aap_hold'.
In most places, the code is careful about ensuring that mbuf
chains referenced by this mechanism are freed only once, but
there is a subtle problem in at_aarpinput(). The code fragment
looks something like:
if (aat->aat_hold != NULL) {
(*ac->ac_if.if_output)(&ac->ac_if, aat->aat_hold, ...);
aat->aat_hold = NULL;
}
The problem here is that when if_output() is called,
aat->aat_hold contains a non-NULL pointer. If the interface
if_output routine calls aarpresolve(), then aarpresolve()
may free aat->aat_hold. This can result in aat_hold being
m_freem'd twice, since if_output will also free it.
The simple solution is to ensure that aat_hold is NULLed out
before calling the if_output routine. The patch below does
this by copying the mbuf pointer into a local variable
so that aat->aat_hold can be NULL when if_output is called.
>How-To-Repeat:
Connecting a FreeBSD router to a busy network seemed to
trigger this problem every few hours, but it should be easy
to construct a sequence of packet arrivals which cause it
directly.
>Fix:
--- aarp.c.orig Fri Mar 24 16:26:08 2000
+++ aarp.c Fri Mar 24 17:48:12 2000
@@ -393,12 +393,20 @@
sizeof( ea->aarp_sha ));
aat->aat_flags |= ATF_COM;
if ( aat->aat_hold ) {
+ struct mbuf *mhold;
+
+ /*
+ * We must NULL out the aat->aat_hold pointer, since otherwise
+ * if_output may call aarpresolve() which could m_freem() it.
+ */
+ mhold = aat->aat_hold;
+ aat->aat_hold = NULL;
+
sat.sat_len = sizeof(struct sockaddr_at);
sat.sat_family = AF_APPLETALK;
sat.sat_addr = spa;
- (*ac->ac_if.if_output)( &ac->ac_if, aat->aat_hold,
+ (*ac->ac_if.if_output)( &ac->ac_if, mhold,
(struct sockaddr *)&sat, NULL); /* XXX */
- aat->aat_hold = 0;
}
}
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200003241806.aa98076>