Re: [RFC] corner cases of open() on procfs symlinks

[Al Viro]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/