Date: Sat, 30 Apr 2011 00:30:29 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: Sergey Kandaurov <pluknet@gmail.com> Cc: freebsd-standards@freebsd.org Subject: Re: O_ACCMODE doesn't respect O_EXEC Message-ID: <20110429223029.GB10840@stack.nl> In-Reply-To: <BANLkTi=Uzo1FE9O0pvdr2zGcf_SAz=auMA@mail.gmail.com> References: <BANLkTi=Uzo1FE9O0pvdr2zGcf_SAz=auMA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Apr 29, 2011 at 04:51:38PM +0400, Sergey Kandaurov wrote:
> As of now, O_ACCMODE is a conjunction of O_{RDONLY,WRONLY,RDWR} flags
> as it was from the beginning. Its definition hasn't changed with
> addition of O_EXEC.
> POSIX states:
> O_ACCMODE - Mask for file access modes.
> O_EXEC falls into this access modes category, so I think O_ACCMODE
> should include O_EXEC.
I agree.
> This may require review of O_ACCMODE usage throughout the kernel, though..
> This simple test demonstrates the problem:
> flags: 0x0
> flags: 0x0
> flags: 0x1
> flags: 0x1
> flags: 0x2
> flags: 0x2
> flags: 0x3ffff
> flags: 0x3
> ^^ decremented due OFLAGS macro that also doesn't respect new access
> mode flag(s)
> ^^ correct version would show 0x40000
Indeed. kern_openat() has a hack that does not apply the FFLAGS macro if
O_EXEC is specified, to work around FFLAGS/OFLAGS not knowing O_EXEC.
Instead, it seems more appropriate to use something like
#define FFLAGS(oflags) (((oflags) & O_EXEC) != 0 ? (oflags) : (oflags) + 1)
#define OFLAGS(fflags) (((fflags) & O_EXEC) != 0 ? (fflags) : (fflags) - 1)
With also the O_ACCMODE change, kern_openat()'s check can be changed to
switch (flags & O_ACCMODE) {
case O_RDONLY:
case O_WRONLY:
case O_RDWR:
case O_EXEC:
break;
default:
return (EINVAL);
}
with an unconditional flags = FFLAGS(flags);
> int
> main(void)
> {
>
> test_with(O_RDONLY);
> test_with(O_WRONLY);
> test_with(O_RDWR);
> test_with(O_EXEC);
>
> return (0);
> }
>
> static void
> test_with(int openflag)
> {
> int fd, flags;
>
> if ((fd = open("file", openflag)) < 0)
> err(1, "open");
> if ((flags = fcntl(fd, F_GETFL)) < 0)
> err(1, "fcntl(fd, GETFL)");
> printf("flags: 0x%x\n", flags);
> printf("flags: 0x%x\n", flags&O_ACCMODE);
> close(fd);
> }
--
Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110429223029.GB10840>