From owner-kdb@oss.sgi.com Thu Aug 17 12:08:34 2000 Received: by oss.sgi.com id ; Thu, 17 Aug 2000 12:08:24 -0700 Received: from pallas.veritas.com ([204.177.156.25]:13253 "EHLO pallas.veritas.com") by oss.sgi.com with ESMTP id ; Thu, 17 Aug 2000 12:08:05 -0700 Received: from megami.veritas.com (megami.veritas.com [192.203.46.101]) by pallas.veritas.com (8.9.1a/8.9.1) with SMTP id MAA03430 for ; Thu, 17 Aug 2000 12:13:23 -0700 (PDT) Received: from saturn.homenet([192.168.225.223]) (980 bytes) by megami.veritas.com via sendmail with P:esmtp/R:smart_host/T:smtp (sender: ) id for ; Thu, 17 Aug 2000 12:07:28 -0700 (PDT) (Smail-3.2.0.101 1997-Dec-17 #4 built 1999-Aug-24) Date: Thu, 17 Aug 2000 20:14:30 +0100 (BST) From: Tigran Aivazian X-Sender: tigran@saturn.homenet To: kdb@oss.sgi.com Subject: [patch] bugfix for kdb Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-kdb@oss.sgi.com Precedence: bulk Return-Path: X-Orcpt: rfc822;kdb-outgoing hi, Rather obvious bugfix. Against kdb v1.3. Regards, Tigran --- vmalloc.c.0 Thu Aug 17 20:12:20 2000 +++ vmalloc.c Thu Aug 17 20:12:40 2000 @@ -39,6 +39,7 @@ { struct vm_struct *vp; + read_lock(&vmlist_lock); for(vp=vmlist; vp; vp = vp->next) { unsigned long end = (unsigned long)vp->addr + vp->size; @@ -47,9 +48,11 @@ if ((starta >= (unsigned long)vp->addr) && (starta < end) && (enda < end)) { + read_unlock(&vmlist_lock); return 1; } } + read_unlock(&vmlist_lock); return 0; } #endif From owner-kdb@oss.sgi.com Thu Aug 17 12:25:44 2000 Received: by oss.sgi.com id ; Thu, 17 Aug 2000 12:25:34 -0700 Received: from farmer8.nanobiz.com ([208.176.9.172]:1043 "EHLO nanobiz.com") by oss.sgi.com with ESMTP id ; Thu, 17 Aug 2000 12:25:29 -0700 Received: from pendragon.eng.nanobiz.com (IDENT:root@pendragon [192.168.1.155]) by nanobiz.com (8.9.3/8.9.3) with ESMTP id MAA21559; Thu, 17 Aug 2000 12:17:04 -0700 From: Scott Lurndal Received: by pendragon.eng.nanobiz.com; Thu, 17 Aug 2000 12:24:57 -0700 Message-Id: <200008171924.MAA07229@pendragon.eng.nanobiz.com> Subject: Re: [patch] bugfix for kdb To: tigran@veritas.com (Tigran Aivazian) Date: Thu, 17 Aug 2000 12:24:57 -0700 (PDT) Cc: kdb@oss.sgi.com In-Reply-To: from "Tigran Aivazian" at Aug 17, 2000 08:14:30 PM X-Mailer: ELM [version 2.5 PL1] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-kdb@oss.sgi.com Precedence: bulk Return-Path: X-Orcpt: rfc822;kdb-outgoing > > hi, > > Rather obvious bugfix. Against kdb v1.3. Not at all obvious for the following reasons: 1) When KDB is active, no other processor by definition is running, thus locking is unnecessary. 2) What if you try to issue a command which uses this function during a breakpoint where the vmlist_lock is already held in write mode? KDB will hang, not a good thing. KDB should never acquire a general kernel lock. There is the risk that a list (such as vmlist) will be in an inconsistent or even broken state, but kdb recovers fairly well when it takes an internal fault. We should probably change this code to not access 'vp' directly, but rather to use kdbgetbytes instead for each element in the vmlist, just like the vm command does. This will preclude faults from following busted pointers. scott > > Regards, > Tigran > > --- vmalloc.c.0 Thu Aug 17 20:12:20 2000 > +++ vmalloc.c Thu Aug 17 20:12:40 2000 > @@ -39,6 +39,7 @@ > { > struct vm_struct *vp; > > + read_lock(&vmlist_lock); > for(vp=vmlist; vp; vp = vp->next) { > unsigned long end = (unsigned long)vp->addr + vp->size; > > @@ -47,9 +48,11 @@ > if ((starta >= (unsigned long)vp->addr) > && (starta < end) > && (enda < end)) { > + read_unlock(&vmlist_lock); > return 1; > } > } > + read_unlock(&vmlist_lock); > return 0; > } > #endif > From owner-kdb@oss.sgi.com Thu Aug 17 12:27:53 2000 Received: by oss.sgi.com id ; Thu, 17 Aug 2000 12:27:43 -0700 Received: from farmer8.nanobiz.com ([208.176.9.172]:2067 "EHLO nanobiz.com") by oss.sgi.com with ESMTP id ; Thu, 17 Aug 2000 12:27:39 -0700 Received: from pendragon.eng.nanobiz.com (IDENT:root@pendragon [192.168.1.155]) by nanobiz.com (8.9.3/8.9.3) with ESMTP id MAA21578; Thu, 17 Aug 2000 12:19:14 -0700 From: Scott Lurndal Received: by pendragon.eng.nanobiz.com; Thu, 17 Aug 2000 12:27:07 -0700 Message-Id: <200008171927.MAA07245@pendragon.eng.nanobiz.com> Subject: Re: [patch] bugfix for kdb To: slurn@nanobiz.com (Scott Lurndal) Date: Thu, 17 Aug 2000 12:27:07 -0700 (PDT) Cc: tigran@veritas.com (Tigran Aivazian), kdb@oss.sgi.com In-Reply-To: <200008171924.MAA07229@pendragon.eng.nanobiz.com> from "Scott Lurndal" at Aug 17, 2000 12:24:57 PM X-Mailer: ELM [version 2.5 PL1] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-kdb@oss.sgi.com Precedence: bulk Return-Path: X-Orcpt: rfc822;kdb-outgoing > > > > > hi, > > > > Rather obvious bugfix. Against kdb v1.3. > > Not at all obvious for the > following reasons: > > 1) When KDB is active, no other processor by definition is running, > thus locking is unnecessary. > > 2) What if you try to issue a command which uses this function > during a breakpoint where the vmlist_lock is already held in > write mode? KDB will hang, not a good thing. > > KDB should never acquire a general kernel lock. There is the > risk that a list (such as vmlist) will be in an inconsistent > or even broken state, but kdb recovers fairly well when it > takes an internal fault. We should probably change this code > to not access 'vp' directly, but rather to use kdbgetbytes instead > for each element in the vmlist, just like the vm command does. This > will preclude faults from following busted pointers. Oops. This code is called by kdbgetbytes to help validate the address. Can't use kdbgetbytes itself. So we will have to live with the possibility (quite slight) that the list is inconsistent when kdbgetbytes calls this function. scott > > scott > > > > > Regards, > > Tigran > > > > --- vmalloc.c.0 Thu Aug 17 20:12:20 2000 > > +++ vmalloc.c Thu Aug 17 20:12:40 2000 > > @@ -39,6 +39,7 @@ > > { > > struct vm_struct *vp; > > > > + read_lock(&vmlist_lock); > > for(vp=vmlist; vp; vp = vp->next) { > > unsigned long end = (unsigned long)vp->addr + vp->size; > > > > @@ -47,9 +48,11 @@ > > if ((starta >= (unsigned long)vp->addr) > > && (starta < end) > > && (enda < end)) { > > + read_unlock(&vmlist_lock); > > return 1; > > } > > } > > + read_unlock(&vmlist_lock); > > return 0; > > } > > #endif > > >