* [PATCH] open: O_DIRECTORY and O_CREAT together should fail
@ 2005-09-23 14:45 Miklos Szeredi
2005-09-23 19:28 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Miklos Szeredi @ 2005-09-23 14:45 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
Add a check for O_DIRECTORY in the O_CREAT path, and return -EINVAL.
Current behavior is inconsistent with documentation:
open(..., O_DIRECTORY|O_CREAT) succeeds if file didn't exist, and
returned descriptor does not refer to a directory.
No other error value quite fits this case:
ENOTDIR - the file doesn't exist, so this is slightly misleading
ENOENT - yes, but we asked for an O_CREAT so why wasn't it created
But EINVAL - invalid combination of flags, is quite good IMO.
Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
Index: linux/fs/namei.c
===================================================================
--- linux.orig/fs/namei.c 2005-09-23 16:35:32.000000000 +0200
+++ linux/fs/namei.c 2005-09-23 16:36:19.000000000 +0200
@@ -1441,6 +1441,9 @@ int open_namei(const char * pathname, in
return error;
goto ok;
}
+ /* O_CREAT | O_DIRECTORY should fail */
+ if (flag & O_DIRECTORY)
+ return -EINVAL;
/*
* Create - we need to know the parent.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-23 14:45 [PATCH] open: O_DIRECTORY and O_CREAT together should fail Miklos Szeredi
@ 2005-09-23 19:28 ` Andrew Morton
2005-09-24 5:52 ` Miklos Szeredi
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2005-09-23 19:28 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-kernel
Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> Add a check for O_DIRECTORY in the O_CREAT path, and return -EINVAL.
>
> Current behavior is inconsistent with documentation:
> open(..., O_DIRECTORY|O_CREAT) succeeds if file didn't exist, and
> returned descriptor does not refer to a directory.
>
> No other error value quite fits this case:
>
> ENOTDIR - the file doesn't exist, so this is slightly misleading
> ENOENT - yes, but we asked for an O_CREAT so why wasn't it created
>
> But EINVAL - invalid combination of flags, is quite good IMO.
>
We could be a bit screwed here. If there are any apps out there which are
using this combination, we just broke them. Essentially the patch is
assuming that nobody is currently using O_CREAT|O_DIRECTORY, but one day in
the future someone will do that.
>
> Index: linux/fs/namei.c
> ===================================================================
> --- linux.orig/fs/namei.c 2005-09-23 16:35:32.000000000 +0200
> +++ linux/fs/namei.c 2005-09-23 16:36:19.000000000 +0200
> @@ -1441,6 +1441,9 @@ int open_namei(const char * pathname, in
> return error;
> goto ok;
> }
> + /* O_CREAT | O_DIRECTORY should fail */
> + if (flag & O_DIRECTORY)
> + return -EINVAL;
>
> /*
> * Create - we need to know the parent.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-23 19:28 ` Andrew Morton
@ 2005-09-24 5:52 ` Miklos Szeredi
2005-09-24 6:09 ` Al Viro
0 siblings, 1 reply; 8+ messages in thread
From: Miklos Szeredi @ 2005-09-24 5:52 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
> > Current behavior is inconsistent with documentation:
> > open(..., O_DIRECTORY|O_CREAT) succeeds if file didn't exist, and
> > returned descriptor does not refer to a directory.
> >
> > No other error value quite fits this case:
> >
> > ENOTDIR - the file doesn't exist, so this is slightly misleading
> > ENOENT - yes, but we asked for an O_CREAT so why wasn't it created
> >
> > But EINVAL - invalid combination of flags, is quite good IMO.
> >
>
> We could be a bit screwed here. If there are any apps out there which are
> using this combination, we just broke them. Essentially the patch is
> assuming that nobody is currently using O_CREAT|O_DIRECTORY, but one day in
> the future someone will do that.
Well yes. But I don't think anybody is using it, and if so they are
clearly breaking the rules in man open(2):
O_DIRECTORY
If pathname is not a directory, cause the open to fail. This
flag is Linux‐specific, and was added in kernel version 2.1.126,
to avoid denial‐of‐service problems if opendir(3) is called on a
FIFO or tape device, but should not be used outside of the
implementation of opendir.
So if someone uses this outside of opendir() and uses it to create a
non-directory, I think they deserve to be screwed.
Miklos
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-24 5:52 ` Miklos Szeredi
@ 2005-09-24 6:09 ` Al Viro
2005-09-24 6:41 ` Miklos Szeredi
0 siblings, 1 reply; 8+ messages in thread
From: Al Viro @ 2005-09-24 6:09 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: akpm, linux-kernel
On Sat, Sep 24, 2005 at 07:52:06AM +0200, Miklos Szeredi wrote:
> Well yes. But I don't think anybody is using it, and if so they are
> clearly breaking the rules in man open(2):
Be liberal in what you accept and all such... Everything else aside,
why bother? This check doesn't buy you anything.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-24 6:09 ` Al Viro
@ 2005-09-24 6:41 ` Miklos Szeredi
2005-09-24 7:01 ` Al Viro
0 siblings, 1 reply; 8+ messages in thread
From: Miklos Szeredi @ 2005-09-24 6:41 UTC (permalink / raw)
To: viro; +Cc: akpm, linux-kernel
> > Well yes. But I don't think anybody is using it, and if so they are
> > clearly breaking the rules in man open(2):
>
> Be liberal in what you accept and all such... Everything else aside,
> why bother?
To conform to well defined semantics?
It just bathers me, that you can get a non-directory file descriptor
with O_DIRECTORY.
Miklos
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-24 6:41 ` Miklos Szeredi
@ 2005-09-24 7:01 ` Al Viro
2005-09-24 7:43 ` Miklos Szeredi
2005-09-24 19:53 ` Kyle Moffett
0 siblings, 2 replies; 8+ messages in thread
From: Al Viro @ 2005-09-24 7:01 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: akpm, linux-kernel
On Sat, Sep 24, 2005 at 08:41:05AM +0200, Miklos Szeredi wrote:
> > > Well yes. But I don't think anybody is using it, and if so they are
> > > clearly breaking the rules in man open(2):
> >
> > Be liberal in what you accept and all such... Everything else aside,
> > why bother?
>
> To conform to well defined semantics?
Well-defined is not exactly the word I'd use for that mess (example -
we still have the last remnant of ancient BSD idiocy in there; the last
case when dangling symlink is still traversed upon object creation,
everything else had been fixed since then).
And O_DIRECTORY is not the only flag that acquires or loses meaning
depending on O_CREAT - consider e.g. O_EXCL. It's a mess, of course,
but this mess is part of userland ABI. We tried to fix symlink idiocy,
BTW, on the assumption that nothing would be relying on it. Didn't
work...
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-24 7:01 ` Al Viro
@ 2005-09-24 7:43 ` Miklos Szeredi
2005-09-24 19:53 ` Kyle Moffett
1 sibling, 0 replies; 8+ messages in thread
From: Miklos Szeredi @ 2005-09-24 7:43 UTC (permalink / raw)
To: viro; +Cc: akpm, linux-kernel
> > > > Well yes. But I don't think anybody is using it, and if so they are
> > > > clearly breaking the rules in man open(2):
> > >
> > > Be liberal in what you accept and all such... Everything else aside,
> > > why bother?
> >
> > To conform to well defined semantics?
>
> Well-defined is not exactly the word I'd use for that mess (example -
> we still have the last remnant of ancient BSD idiocy in there; the last
> case when dangling symlink is still traversed upon object creation,
> everything else had been fixed since then).
>
> And O_DIRECTORY is not the only flag that acquires or loses meaning
> depending on O_CREAT - consider e.g. O_EXCL. It's a mess, of course,
> but this mess is part of userland ABI. We tried to fix symlink idiocy,
> BTW, on the assumption that nothing would be relying on it. Didn't
> work...
OK, I'm convinced.
Miklos
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] open: O_DIRECTORY and O_CREAT together should fail
2005-09-24 7:01 ` Al Viro
2005-09-24 7:43 ` Miklos Szeredi
@ 2005-09-24 19:53 ` Kyle Moffett
1 sibling, 0 replies; 8+ messages in thread
From: Kyle Moffett @ 2005-09-24 19:53 UTC (permalink / raw)
To: Al Viro; +Cc: Miklos Szeredi, akpm, linux-kernel
On Sep 24, 2005, at 03:01:50, Al Viro wrote:
> And O_DIRECTORY is not the only flag that acquires or loses meaning
> depending on O_CREAT - consider e.g. O_EXCL. It's a mess, of
> course, but this mess is part of userland ABI. We tried to fix
> symlink idiocy, BTW, on the assumption that nothing would be
> relying on it. Didn't work...
Maybe CONFIG_FIX_CRAPPY_ABI_CORNER_CASES? If the user is willing to
deal with some minimal breakage and fix programs relying on icky
unsupported behavior, then they could turn that on for a slightly
more secure system. Make it depend on "experimental" and give it big
warning messages. It's likely that some of the more-secure server-
oriented distros that run patched gcc and such to avoid buffer
overflow and such might turn it on.
Cheers,
Kyle Moffett
--
Simple things should be simple and complex things should be possible
-- Alan Kay
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2005-09-24 19:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-09-23 14:45 [PATCH] open: O_DIRECTORY and O_CREAT together should fail Miklos Szeredi
2005-09-23 19:28 ` Andrew Morton
2005-09-24 5:52 ` Miklos Szeredi
2005-09-24 6:09 ` Al Viro
2005-09-24 6:41 ` Miklos Szeredi
2005-09-24 7:01 ` Al Viro
2005-09-24 7:43 ` Miklos Szeredi
2005-09-24 19:53 ` Kyle Moffett
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.