>
>
> On Sun, 20 Jun 1999, Alexander Viro wrote:
> >
> > Good. The only question being: why are we pushing the traditional symlinks
> > under the same roof? That's what makes me very uncomfortable. And it makes
> > lookup_dentry() pretty uncomfortable too - we got *way* too nasty
> > recursion there.
>
> I much prefer recursion. Recursion is cheap and simple, and optimizes the
> right case. And we can (and do) easily limit it, so there is no real stack
Huh? You mean that the right case here being *not* the regular symlink?
> space issue, espcially as the lookup is pretty frugal when it comes to
> stack space anyway).
Ahem... Take a look at coda_follow_link(), please. Even if we want to keep
recursion here (I don't have iterative version anyway) excluding the
foo_follow_link() from it reduces the stack wastage by factor of 10.
Literally. And removes a lot of code duplication.
I'm not saying that old mechanism should be dropped/changed/whatever. Just
that the common case (right now *everything* except proc/<pid>/* stuff)
asks for treating it separately. With lookup_dentry() called from
do_follow_link(), not from foo_follow_link().
> A lot of people think recursion is "nasty" - while in fact it's a very
> elegant way to do something that would otherwise be much uglier. So I
Oh, recursion per se is completely OK with me (heck, I worked with
functional languages more than enough). But there are ways to make it ugly
and that's what happens here. It's not the fact that we have a recursive
lookup_dentry(), it's bringing tons of cruft into stack between the nested
calls and severe code duplication. And multiple pathes of recursion (every
foo_follow_link contains one).
Linus, for *every* function that was copied from minixfs to later
filesystems I can point to the place where we had a bug just because the
fix/change didn't propagate to some drivers. Code duplication is inevitable,
but in this case we are asking for future troubles for no good reason.
> think it's only right and proper to have symlinks be just another kind of
> wormhole. No special cases.
Up to you. But IMO you are mixing two seriously different objects
together and the cost is pretty high. Please, look at the implementations.
coda_follow_link() eats a bloody kilobyte of stack. With the patch I've
posted (old variant) we are getting <100 bytes for each recursive call.
It's order of magnitude. I'm more than sure that with sufficiently long
and perverted code path (we have them) the current implementation can be
used to overflow the ring 0 stack.
Alternative variant being generic_readlink() and generic_follow_link()
that would be usable for symlinks that provide ->k_readlink(). Something
definitely should be done - stack smashing in ring 0 is Bad Thing(tm).
PS: ObCodeDuplication: sys_mmap() on PPC. down(¤t->mm->mmap_sem)
missing. fcheck() instead of fget(). The former is more or less fresh, but
the latter is about 1.5 years old. Exploitable race. Sigh... And there are
other similar buggers. grep(1) is our friend...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/