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>