bug in open bsd

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug in open bsd

Amit Kubovski
Hi,



 



While testing the regex of open bsd I have found the following bug:



 



In regexec.c you will find the lines



 



            if (g->nstates <= CHAR_BIT*sizeof(states1) && !(eflags&REG_LARGE))



                        return(smatcher(g, (char *)string, nmatch, pmatch, eflags));



            else



                        return(lmatcher(g, (char *)string, nmatch, pmatch, eflags));



}          



 



But states1 is defined as follows



#define  states1 states               /* for later use in regexec() decision */



 



Note that the above lines appear after doing



 



#define  states   char *



 



Therefore states1 is now equal to char*



 



Now the sizeof char* on 64 bit operating system environments is 8 bytes, but the size of states being long for the short matcher might be only 4 bytes (if using Microsoft's compilers) and therefore a bug is created.



 



This bug will appear if compiling this code using a Microsoft compiler and running on x64 or itanium machine.



 



Amit


Reply | Threaded
Open this post in threaded view
|

Re: bug in open bsd

Miod Vallat
> In regexec.c you will find the lines
>
>             if (g->nstates <= CHAR_BIT*sizeof(states1) && !(eflags&REG_LARGE))
>                         return(smatcher(g, (char *)string, nmatch, pmatch,
eflags));
>             else
>                         return(lmatcher(g, (char *)string, nmatch, pmatch,
eflags));
> }
>
> But states1 is defined as follows
>
> #define  states1 states               /* for later use in regexec() decision
*/

>
> Note that the above lines appear after doing
>
> #define  states   char *
>
> Therefore states1 is now equal to char*
>
> Now the sizeof char* on 64 bit operating system environments is 8 bytes, but
> the size of states being long for the short matcher might be only 4 bytes (if
> using Microsoft's compilers) and therefore a bug is created.

Although this is indeed not what the initial author intended, this is not
important since we do not support systems where sizeof(void *) < sizeof(long).

> This bug will appear if compiling this code using a Microsoft compiler and
> running on x64 or itanium machine.

This is someone else's problem. Running with 32 bit longs and 64 bit pointers
is asking for much more problems than this one.

Miod

Reply | Threaded
Open this post in threaded view
|

Re: bug in open bsd

Theo de Raadt
In reply to this post by Amit Kubovski
> This bug will appear if compiling this code using a Microsoft
> compiler and running on x64 or itanium machine.

You are talking about a particular compiler environment using 32 bit
longs and 64 bit pointers.

Our source tree is (currently) only safe for two type storage models:

        ILP32
and
        I32LP64

Thus, pointer and long are always the same.

Making our source tree safe for anything more than these models is a
nightmare.

Actually, making anyone's source tree utterly safe for other models
(such as IL32P64 or ILP64) is a quick way to go completely insane.
Some smaller problems are easily fixed, but many can only be solved by
using lots of #ifdef's... which is just another way of saying the code
gets worse.

The I32LP64 model was chosen because it was considered a safer
transition from ILP32 than any of the other models (with caveats, for
instance, big-endian I32LP64 has some very different behaviours --
powerful or nasty depending on your perpsective).

Please also note that we do not have any architectures where we would
run with such a model (well, I mean, where we would choose to run with
such a model).  If we ever run on Itanium, we will most definately be
using them as I32LP64.  Even if this is sometimes a tiny bit less
efficient, it is safe.

I32LP64 is more robust than any of the other choices, by far.

Reply | Threaded
Open this post in threaded view
|

Re: bug in open bsd

Daniel Hartmeier
In reply to this post by Amit Kubovski
On Tue, Aug 29, 2006 at 08:13:39AM +0200, Amit Kubovski wrote:

> But states1 is defined as follows
> #define  states1 states               /* for later use in regexec() decision */
>
> Note that the above lines appear after doing
> #define  states   char *
>
> Therefore states1 is now equal to char*

Not in OpenBSD's /usr/src/lib/libc/regex/regexec.c

/*      $OpenBSD: regexec.c,v 1.11 2005/08/05 13:03:00 espie Exp $ */

#define states  long
#define states1 states          /* for later use in regexec() decision */

Daniel

Reply | Threaded
Open this post in threaded view
|

Re: bug in open bsd

Miod Vallat
> Not in OpenBSD's /usr/src/lib/libc/regex/regexec.c
>
> /*      $OpenBSD: regexec.c,v 1.11 2005/08/05 13:03:00 espie Exp $ */
>
> #define states  long
> #define states1 states          /* for later use in regexec() decision */

Look ~50 lines below.

Miod