On Thu, Jun 06, 2013 at 10:38:31AM +0900, Linus Torvalds wrote:
> On Thu, Jun 6, 2013 at 10:20 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> > I'm not sure whether to treat that as a bug or as a weird misfeature
> > enshrined in userland ABI:
> > open("/tmp", O_CREAT, 0) => -EISDIR // LAST_NORM case
> > open("/", O_CREAT, 0) => -EISDIR // LAST_ROOT
> > open(".", O_CREAT, 0) => -EISDIR // LAST_DOT
> > open("..", O_CREAT, 0) => -EISDIR // LAST_DOTDOT
> > open("/proc/self/cwd", O_CREAT, 0) => success // LAST_BIND
> > open("/proc/self/cwd/", O_CREAT, 0) => -EISDIR // trailing slashes
>
> Ok, that looks buggy. O_CREAT should definitely return EISDIR for
> /proc/self/cwd too, since it's a directory. I don't think the
> O_RDWR/O_WRONLY thing should matter.
>
> > I would obviously
> > like to do that - do_last() is far too convoluted as it is; the only
> > question is whether we can change the first weirdness... Comments?
>
> Exactly which cases does that change? I have no objections if it's
> only the "LAST_BIND" case that now starts returning EISDIR. Is there
> anything else it affects?
LAST_BIND gets to go through the EISDIR and ENOTDIR checks that way, which
fixes these two bugs.
LAST_DOT/LAST_DOTDOT/LAST_ROOT end up checking whether we are at the
directory or not; sure, we know that we are, so these tests are
redundant, but I really don't think it's worth optimizing for. We are
not generating any data misses and arguably we reduce instruction cache
footprint a bit, not that it would be noticable with the I$ horror
do_last() still is...
What really happens in that switch is that do_last() tries to be too smart
and ends up skipping a few things too many.
> That said, obviously if something breaks, we'd have to revert it, and
> as a cleanup rather than some serious bug (ie this doesn't cause
> crashes or security issues), I suspect this should wait until 3.11
> regardless. No?
Probably... procfs symlinks neutering O_DIRECTORY might, in theory, be usable
to cook something nasty, but I don't see any obvious ways to exploit that.
FWIW, resulting kernel seems to survive the minimal beating, but obviously
more is needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
