Date: Tue, 04 Jun 2002 00:31:41 +0200 From: Andre Oppermann <oppermann@pipeline.ch> To: Archie Cobbs <archie@dellroad.org> Cc: freebsd-net@freebsd.org Subject: Re: Race condition with M_EXT ref count? Message-ID: <3CFBEE4D.80363D65@pipeline.ch> References: <200206031913.g53JD7547163@arch20m.dellroad.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Archie Cobbs wrote: > > This is a question about M_EXT mbuf reference counts in FreeBSD-stable. > > There are several instances in kern/uipc_mbuf.c that add a reference > to an M_EXT mbuf by either incrementing the entry in the mclrefcnt[] > array or invoking the "custom" ext_ref routine. > > However, it seems that these instances are all broken because they > don't wrap these operations within splimp()... > > Isn't the following C statement *not* atomic? > > mclrefcnt[mtocl(m->m_ext.ext_buf)]++; > > And isn't access to mclrefcnt[] supposed to be protected by splimp()? > Note: MCLFREE() *does* set splimp() before decrementing M_EXT ref counts. > > Therefore, isn't there a race condition wrt. the M_EXT reference counts? Yes, this looks dangerous. If you get hit by an interrupt the thing could change under you. Part of this has been discussed a couple of month ago in a thread about 64bit interface counter and atomic updates for (ref)counters in the network stack. Also patches implementing atomic counters for this purpose and performace numbers have been postet within that thread. The best would be to dig that up. I think the subject was along the lines of "64bit counters" or so. There are a couple of other places which look suspicious in similiar regard. There are some suspicius instances of direct rt_refcnt-- decrements. Normally this should be done via RTFREE() so that routes could end up with refcount zero without getting freed. Also sometimes the function rtfree() is called directly instead of the macro RTFREE as in the vast majority of the places. If time permits I'll look into these cases. -- Andre > The functions which fail to set splimp() before adding a reference are: > > m_copym() > m_copypacket() > m_split() > > Thanks for any comments/clarification on this subject.. > > -Archie > > __________________________________________________________________________ > Archie Cobbs * Packet Design * http://www.packetdesign.com > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-net" in the body of the message To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3CFBEE4D.80363D65>