[5453] in bugtraq
Re: Possible SERIOUS bug in open()?
daemon@ATHENA.MIT.EDU (Tim Newsham)
Sat Oct 25 16:25:47 1997
Date: Sat, 25 Oct 1997 09:26:12 -1000
Reply-To: Tim Newsham <newsham@ALOHA.NET>
From: Tim Newsham <newsham@ALOHA.NET>
X-To: mem@MV.MV.COM
To: BUGTRAQ@NETSPACE.ORG
In-Reply-To: <199710250309.XAA17853@mv.mv.com> from "Mark E. Mallett" at Oct
24, 97 11:09:34 pm
> > + if(!flags)
> > + flags++;
> > +
>
> This will only cover the -1 case. Perhaps also changing:
>
> > flags = FFLAGS(uap->flags);
>
> to
>
> > flags = FFLAGS(uap->flags) & 3;
>
> and then the zero test as above?
What about the higher flag bits? (O_APPEND, O_CREAT, ...)
What FFLAGS is supposed to do is remap the low two bits:
O_RDONLY 0 -> FREAD 1
O_WRONLY 1 -> FWRITE 2
O_RDWR 2 -> FREAD | FWRITE 3
this would be much more clear and much less error-prone if
this was done explicitely instead of with a clever hack that
obfuscates the operation. Implementing it in this way
makes it clear what to do:
switch(uap->flags & O_ACCMODE) {
case O_RDONLY:
lowbits = FREAD;
break;
case O_WRONLY:
lowbits = FWRITE;
break;
case O_RDWR:
lowbits = FREAD | FWRITE;
break;
default:
return EINVAL;
}
flags = (uap->flags & ~O_ACCMODE) | lowbits;
yes this will mean that open and F_SETFL will be slightly
slower, but it is probably not noticeable, and the extra
clarity is worth it.
> -mm-
Tim N.
