From owner-freebsd-hackers@FreeBSD.ORG Fri Jan 3 17:40:20 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 8470F1A5; Fri, 3 Jan 2014 17:40:20 +0000 (UTC) Received: from mx11.netapp.com (mx11.netapp.com [216.240.18.76]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 5F04611A8; Fri, 3 Jan 2014 17:40:19 +0000 (UTC) X-IronPort-AV: E=Sophos;i="4.95,598,1384329600"; d="scan'208";a="93491130" Received: from vmwexceht06-prd.hq.netapp.com ([10.106.77.104]) by mx11-out.netapp.com with ESMTP; 03 Jan 2014 09:40:19 -0800 Received: from SACEXCMBX04-PRD.hq.netapp.com ([169.254.6.58]) by vmwexceht06-prd.hq.netapp.com ([10.106.77.104]) with mapi id 14.03.0123.003; Fri, 3 Jan 2014 09:40:19 -0800 From: "Gumpula, Suresh" To: Alfred Perlstein , Julian Elischer , "freebsd-hackers@freebsd.org" Subject: RE: Reference count race window Thread-Topic: Reference count race window Thread-Index: AQHPCA0ePlCdkUTb2Eq8swVe1xATjJpynbOA//98gUCAAJ/7AP//hPzAgACUt4CAAGawEA== Date: Fri, 3 Jan 2014 17:40:18 +0000 Message-ID: References: <52C5ED3E.4020805@mu.org> <52C5F8A3.9000902@freebsd.org> <52C61088.3080703@mu.org> <52C62617.7020304@mu.org> In-Reply-To: <52C62617.7020304@mu.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.106.53.53] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jan 2014 17:40:20 -0000 Thanks a lot for suggestions/comments Alfred/Julian/Poul Henning. I will= instrument as per Alfred suggestion first and will see if misuse of crfree= is the root cause of corruption than a race window. By the way , what are all the ways we have in freebsd to debug memory corr= uptions. I am aware of options DEBUG_REDZONE( overflow detection) , DEB= UG_MEMGAURD( for a malloc type), MALLOC_DEBUG_MAXZONES and some INVARIANT= S in=20 Kern_malloc.c which caches the malloc_type that most recently freed . A= nd any more corruption debugging methods we already have ? Thanks Suresh -----Original Message----- From: Alfred Perlstein [mailto:bright@mu.org]=20 Sent: Thursday, January 02, 2014 9:53 PM To: Gumpula, Suresh; Julian Elischer; freebsd-hackers@freebsd.org Subject: Re: Reference count race window On 1/2/14, 6:38 PM, Gumpula, Suresh wrote: > Hi Alfred, > I agree that there could have been an extra/invalid crfree() whic= h decremented the count and looks valid crhold(acquire) from socket code= panic'ed in my case. As per your suggestion if we > replace the assert with if condition in release, we will end up panic= ing when the actual crfree() happens. But we may not be knowing who crfr= ee'ed() in the first and invalid place. Am I correct? I will try your su= ggestion. > > Can you please bit more explain your array trick ? I think the simplest thing to do would be to replace crfree with a macro to= pass __FILE__ and __LINE__ down into the actual crfree (or you can use bui= ltin_return_address to get the stack address of the caller instead. Then just either have an array of {file,line} tuples or instead just return= addresses in the struct. Just extend struct ucred and add this: #define MAX_PREV 10 const char *files[MAX_PREV]; int lines[MAX_PREV]; Then hack your own version of refcount_release(), call it refcount_release2() but have it take a pointer to an integer that it will w= rite the value of the old refcount into. Then you can use that return value like so: if (old_refcount < MAX_PREV) { cred->files[old_refcount] =3D pointer_to_file_name; cred->lines[old_refcount] =3D line_number; } Then add the assert. or better yet, turn on INVARIANTS (or maybe try each = option in turn as INVARIANTS might hide the bug). When you crash you can then see the last few callers who did free inside th= e struct. Since the refcount "old_refcount" is atomically manipulated you should see = the last few frees that send you negative. -Alfred > > Thanks > Suresh > > > > -----Original Message----- > From: owner-freebsd-hackers@freebsd.org=20 > [mailto:owner-freebsd-hackers@freebsd.org] On Behalf Of Alfred=20 > Perlstein > Sent: Thursday, January 02, 2014 8:21 PM > To: Gumpula, Suresh; Julian Elischer; freebsd-hackers@freebsd.org > Subject: Re: Reference count race window > > > On 1/2/14, 3:53 PM, Gumpula, Suresh wrote: >>>> Without changing the return-value semantics of refcount_acquire, we=20 >>>> have introduced a panic if we detected a race as below. >>>> static __inline void >>>> refcount_acquire(volatile u_int *count) { >>>> u_int old; >>>> >>>> old =3D atomic_fetchadd_int(count, 1); >>>> if (old =3D=3D 0) { >>>> panic("refcount_acquire race condition detected!\n"); >>>> } >>>>>>> so what is the stacktrace of the panic? >> It's from the socket code calling crhold. It's a non debug build( NO I= NVARIANTS ) >> >> #4 0xffffffff80331d34 in panic (fmt=3D0xffffffff805c1e60=20 >> "refcount_acquire race condition detected!\n") at >> ../../../../sys/kern/kern_shutdown.c:1009 >> #5 0xffffffff80326662 in refcount_acquire (count=3D) at >> ../../../../sys/sys/refcount.h:65 >> #6 crhold (cr=3D) at >> ../../../../sys/kern/kern_prot.c:1814 >> #7 0xffffffff803aa0d9 in socreate (dom=3D,=20 >> aso=3D0xffffff80345c1b00, type=3D, proto=3D0,=20 >> cred=3D0xffffff0017d7aa00, td=3D0xffffff000b294410) at >> ../../../../sys/kern/uipc_socket.c:441 >> #8 0xffffffff803b2e5c in socket (td=3D0xffffff000b294410, >> uap=3D0xffffff80345c1be0) at ../../../../sys/kern/uipc_syscalls.c:201 >> #9 0xffffffff80539ecb in syscall (frame=3D0xffffff80345c1c80) at >> ../../../../sys/amd64/amd64/trap.c:1260 >> > If it's a non-debug build then how do you know that someone isn't incorre= ctly lowering the refcount? > > Please try some invariants or at least manually turn on the one KASSERT I= mentioned. > > Another trick would be to add a an array of char*+int for the last few pl= aces that decremented, you can use the returned refcount as an index to tha= t array to track who may be doing the extra frees. > > -Alfred > > _______________________________________________ > freebsd-hackers@freebsd.org mailing list=20 > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org= " >