Mailing-List: contact cygwin-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-subscribe AT sources DOT redhat DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin/>
List-Post: <mailto:cygwin AT sources DOT redhat DOT com>
List-Help: <mailto:cygwin-help AT sources DOT redhat DOT com>, <http://sources.redhat.com/ml/#faqs>
Sender: cygwin-owner AT sources DOT redhat DOT com
Delivered-To: mailing list cygwin AT sources DOT redhat DOT com
Message-ID: <00e501c0ff14$6ad8dd40$806410ac@local>
From: "Robert Collins" <robert DOT collins AT itdomain DOT com DOT au>
To: <cygwin AT cygwin DOT com>, <cygwin-patches AT cygwin DOT com>
References: <EA18B9FA0FE4194AA2B4CDB91F73C0EF08F09E AT itdomain002 DOT itdomain DOT net DOT au> <20010627012932 DOT I19058 AT redhat DOT com> <20010627013502 DOT K19058 AT redhat DOT com> <008201c0ff0d$8fe3c2a0$806410ac AT local> <009701c0ff0e$4d796400$806410ac AT local>
Subject: Re: pthreads works, sorta - got it
Date: Thu, 28 Jun 2001 00:21:11 +1000
MIME-Version: 1.0
Content-Type: multipart/mixed;
	boundary="----=_NextPart_000_00E2_01C0FF68.3BCDD240"
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook Express 5.50.4522.1200
X-MimeOLE: Produced By Microsoft MimeOLE V5.50.4522.1200
X-OriginalArrivalTime: 27 Jun 2001 14:10:37.0004 (UTC) FILETIME=[F02848C0:01C0FF12]

------=_NextPart_000_00E2_01C0FF68.3BCDD240
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

Got the bug... attached is a correct patch that doesn't break anything
AFAICT.

GCC was optimising the variable access when the macro
check_null_empty-path_errno was used, and accessing the memory area _before_
the readcheck! The overhead of a proper function should be lower than that
of VirtualQueryHowever, so I've made it a function. It could have the guts
of check_null_empty_path copied into it for speed, but that's optional IMO.

Wed Jun 27 23:30:00 2001  Robert Collins <rbtcollins AT hotmail DOT com>

    * path.h  (check_null_empty_path_errno): Convert to a function
prototype.
    * path.cc (check_null_empty_path_errno): New function.
    (check_null_empty_path): Change from VirtualQuery to IsBadWritePtr.
    * resource.cc (getrlimit): Ditto.
    (setrlimit): Ditto.
    * thread.cc (check_valid_pointer): Ditto.


----- Original Message -----
From: "Robert Collins" <robert DOT collins AT itdomain DOT com DOT au>
To: "Robert Collins" <robert DOT collins AT itdomain DOT com DOT au>; <cygwin AT cygwin DOT com>;
<cygwin-patches AT cygwin DOT com>
Sent: Wednesday, June 27, 2001 11:37 PM
Subject: Re: pthreads works, sorta


> The last patch was bad - sorry! (path.cc had a copy-n-pasto).
>
> Rob
> ----- Original Message -----
> From: "Robert Collins" <robert DOT collins AT itdomain DOT com DOT au>
> To: <cygwin AT cygwin DOT com>; <cygwin-patches AT cygwin DOT com>
> Sent: Wednesday, June 27, 2001 11:32 PM
> Subject: Re: pthreads works, sorta
>
>
> > changelog:
> >
> > Wed Jun 27 23:30:00 2001  Robert Collins <rbtcollins AT hotmail DOT com>
> >
> >     * path.cc (check_null_empty_path): Change from VirtualQuery to
> > IsBadWritePtr.
> >     * resource.cc (getrlimit): Ditto.
> >     (setrlimit): Ditto.
> >     * thread.cc (check_valid_pointer): Ditto.
> >
> >
> > What about the other instances of virtualQuery? Or are the appropriate..
> >
> > Rob (Your humble delegate).
> >
> >
> >
> > ----- Original Message -----
> > From: "Christopher Faylor" <cgf AT redhat DOT com>
> > To: <cygwin AT cygwin DOT com>
> > Sent: Wednesday, June 27, 2001 3:35 PM
> > Subject: Re: pthreads works, sorta
> >
> >
> > > On Wed, Jun 27, 2001 at 01:29:32AM -0400, Christopher Faylor wrote:
> > > >On Wed, Jun 27, 2001 at 01:10:35PM +1000, Robert Collins wrote:
> > > >>> -----Original Message-----
> > > >>> From: Greg Smith [mailto:gsmith AT nc DOT rr DOT com]
> > > >>
> > > >>>
> > > >>> More experimenting with my home computer, dual pIII 850:
> > > >>>
> > > >>> 1. 117  157 328
> > > >>> 2. 822 1527 ---
> > > >>> 3. 194  240 453
> > > >>> 4. 169  181 516
> > > >>>
> > > >>As usual, I write a missive, then solve the puzzle.
> > > >>
> > > >>try this:
> > > >>
> > > >>
> > > >>int __stdcall
> > > >>check_valid_pointer (void *pointer)
> > > >>{
> > > >>  if (!pointer || IsBadWritePtr(pointer, sizeof
(verifyable_object)))
> > > >>    return EFAULT;
> > > >>  return 0;
> > > >>}
> > > >
> > > >This is not quite the same thing as VirtualQuery.  This verifies that
> the
> > > >process can write to memory.  It doesn't verify that it is
accessible.
> > > >
> > > >Maybe that is not important but I would have to think about this.
> > > >
> > > >Nice find, though, Rob.
> > >
> > > I've thought about it.  IsBadWritePtr should be fine in both
> > check_null_empty_path
> > > and check_valid_pointer.
> > >
> > > Could you submit a patch, Rob?  If you are motivated, I'd appreciate a
> > cleanup
> > > patch for resource.cc, too.
> > >
> > > cgf
> > >
> > > --
> > > Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
> > > Bug reporting:         http://cygwin.com/bugs.html
> > > Documentation:         http://cygwin.com/docs.html
> > > FAQ:                   http://cygwin.com/faq/
> > >
> > >
> >
>

------=_NextPart_000_00E2_01C0FF68.3BCDD240
Content-Type: application/octet-stream;
	name="virtualquery.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="virtualquery.patch"

? virtualquery.patch=0A=
Index: path.cc=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/src/src/winsup/cygwin/path.cc,v=0A=
retrieving revision 1.154=0A=
diff -u -p -r1.154 path.cc=0A=
--- path.cc	2001/06/24 22:26:52	1.154=0A=
+++ path.cc	2001/06/27 14:16:28=0A=
@@ -3357,14 +3357,24 @@ cygwin_split_path (const char *path, cha=0A=
 int __stdcall=0A=
 check_null_empty_path (const char *name)=0A=
 {=0A=
-  MEMORY_BASIC_INFORMATION m;=0A=
-  if (!name || !VirtualQuery (name, &m, sizeof (m)) || (m.State !=3D =
MEM_COMMIT))=0A=
-    return EFAULT;=0A=
+  if (!name || IsBadStringPtr (name, MAX_PATH + 1))=0A=
+    {=0A=
+      return EFAULT;=0A=
+    }=0A=
 =0A=
   if (!*name)=0A=
     return ENOENT;=0A=
 =0A=
   return 0;=0A=
+}=0A=
+=0A=
+int __stdcall=0A=
+check_null_empty_path_errno(const char *name)=0A=
+{ =0A=
+  int __err; =0A=
+  if ((__err =3D check_null_empty_path (name))) =0A=
+    set_errno (__err); =0A=
+  return __err; =0A=
 }=0A=
 =0A=
 =
/************************************************************************=
*****/=0A=
Index: path.h=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/src/src/winsup/cygwin/path.h,v=0A=
retrieving revision 1.30=0A=
diff -u -p -r1.30 path.h=0A=
--- path.h	2001/06/24 22:26:52	1.30=0A=
+++ path.h	2001/06/27 14:16:28=0A=
@@ -151,20 +151,12 @@ class path_conv=0A=
 int __stdcall get_device_number (const char *name, int &unit, BOOL =
from_conv =3D FALSE)  __attribute__ ((regparm(3)));=0A=
 int __stdcall slash_unc_prefix_p (const char *path) __attribute__ =
((regparm(1)));=0A=
 int __stdcall check_null_empty_path (const char *name) __attribute__ =
((regparm(1)));=0A=
+int __stdcall check_null_empty_path_errno (const char *name) =
__attribute__ ((regparm(1)));=0A=
 =0A=
 const char * __stdcall find_exec (const char *name, path_conv& buf, =
const char *winenv =3D "PATH=3D",=0A=
 			int null_if_notfound =3D 0, const char **known_suffix =3D NULL)  =
__attribute__ ((regparm(3)));=0A=
 =0A=
 /* Common macros for checking for invalid path names */=0A=
-=0A=
-#define check_null_empty_path_errno(src) \=0A=
-({ \=0A=
-  int __err; \=0A=
-  if ((__err =3D check_null_empty_path(src))) \=0A=
-    set_errno (__err); \=0A=
-  __err; \=0A=
-})=0A=
-=0A=
 #define isdrive(s) (isalpha (*(s)) && (s)[1] =3D=3D ':')=0A=
 =0A=
 static inline bool=0A=
Index: resource.cc=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/src/src/winsup/cygwin/resource.cc,v=0A=
retrieving revision 1.10=0A=
diff -u -p -r1.10 resource.cc=0A=
--- resource.cc	2001/06/24 22:26:52	1.10=0A=
+++ resource.cc	2001/06/27 14:16:29=0A=
@@ -105,8 +105,7 @@ extern "C"=0A=
 int=0A=
 getrlimit (int resource, struct rlimit *rlp)=0A=
 {=0A=
-  MEMORY_BASIC_INFORMATION m;=0A=
-  if (!rlp || !VirtualQuery (rlp, &m, sizeof (m)) || (m.State !=3D =
MEM_COMMIT))=0A=
+  if (!rlp || IsBadWritePtr (rlp, sizeof (struct rlimit)))=0A=
     {=0A=
       set_errno (EFAULT);=0A=
       return -1;=0A=
@@ -141,8 +140,7 @@ extern "C"=0A=
 int=0A=
 setrlimit (int resource, const struct rlimit *rlp)=0A=
 {=0A=
-  MEMORY_BASIC_INFORMATION m;=0A=
-  if (!rlp || !VirtualQuery (rlp, &m, sizeof (m)) || (m.State !=3D =
MEM_COMMIT))=0A=
+  if (!rlp || IsBadWritePtr ((void *) rlp, sizeof (struct rlimit)))=0A=
     {=0A=
       set_errno (EFAULT);=0A=
       return -1;=0A=
Index: thread.cc=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v=0A=
retrieving revision 1.38=0A=
diff -u -p -r1.38 thread.cc=0A=
--- thread.cc	2001/06/26 14:57:33	1.38=0A=
+++ thread.cc	2001/06/27 14:16:30=0A=
@@ -739,9 +739,7 @@ verifyable_object::~verifyable_object ()=0A=
 int __stdcall=0A=
 check_valid_pointer (void *pointer)=0A=
 {=0A=
-  MEMORY_BASIC_INFORMATION m;=0A=
-  if (!pointer || !VirtualQuery (pointer, &m, sizeof (m))=0A=
-      || (m.State !=3D MEM_COMMIT))=0A=
+  if (!pointer || IsBadWritePtr(pointer, sizeof (verifyable_object)))=0A=
     return EFAULT;=0A=
   return 0;=0A=
 }=0A=


------=_NextPart_000_00E2_01C0FF68.3BCDD240
Content-Type: text/plain; charset=us-ascii

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/
------=_NextPart_000_00E2_01C0FF68.3BCDD240--