lkml.org 
[lkml]   [1998]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: symlinks.

On Tue, 14 Apr 1998, C. Scott Ananian wrote:
> On Tue, 14 Apr 1998, C. Scott Ananian wrote:
>
> > Regarding the hullaballo about stack usage on follow_symlink depth limits:
> > From my reading of the code, it does not seem as if do_follow_symlink is
> > recursive at all. It is called in an for(ever) loop in lookup_dentry()
> > in fs/namei.c; it only goes down one link per invocation, and pushes
> > nothing onto the stack.
>
> OK, I was wrong. The calls are recursive: lookup_dentry() calls
> do_follow_link() which (in the filesystem-dependent code) calls
> lookup_dentry again, which will call do_follow_link() again. So it is
> recursive. BUT IT DOESN'T HAVE TO BE.
>
> I'm like to change the semantics so that the filesystem-dependant
> follow_symlink() function calls lookup_dentry with follow=0.
> We could then replace the recursion with a loop in do_follow_symlink().

Hmm... comments in the code (fs/namei.c) claim:

* The new code replaces the old recursive symlink resolution with
* an iterative one (in case of non-nested symlink chains). It does
* this by looking up the symlink name from the particular filesystem,
* and then follows this name as if it were a user-supplied one. This
* is done solely in the VFS level, such that <fs>_follow_link() is not
* used any more and could be removed in future. As a side effect,
* dir_namei(), _namei() and follow_link() are now replaced with a single
* function lookup_dentry() that can handle all the special cases of the former
* code.

So it looks like lookup_dentry is written with iterative behavior in
mind, but if your analysis of the code is right, then do_follow_link
is still recursing, which it shouldn't. (quick check of ext2 code -
yup.) So we should be able to get this behavior by simply changing
lookup_dentry(x,y,1) to lookup_dentry(x,y,0) in individual
<fs>_follow_link()'s, right?

Looks like AFFS, Coda, ext2, ISO, minix, NFS, proc. romfs, sysv, and
ufs implement follow_link's...

It's also seems odd (IMHO) that the comments claim the link-following
is iterative, but the link-limit is enforced in follow_link (which
appears to have been done with the old recursive scheme in mind.) It
seems to me the "for(;;)" in lookup_dentry() should be replaced with a
link-limiting loop instead.

Have _I_ missed something?

> We could then use Tarjan's algorithm to accurately detect loops without
> false-positives.

That's the one with the "chaser" that advances once for every two
advances of the "real", right? (detects loop within 2n?)

> I hope I haven't screwed anything up this time. Comments?

Don't know if Linus would accept this at the moment (given the code
freeze), since it changes the semantics of inode->i_op->follow_link().
But (IMVVVHO) it's The Right Thing To Do - it removes recursion from
the kernel (which is stack-limited) and opens up both greater
configurability (allowing ambiguous depth won't overflow the stack)
and the ability to do real loop detection.

It also would (I think) bring the code into alignment with the
documentation, although we all know the proverb about when the code
and docs disagree ;-)

Adam
--
You crucify all honesty \\Adam D. Bradley artdodge@cs.bu.edu
No signs you see do you believe \\Boston University Computer Science
And all your words just twist and turn\\ Grad Student and Linux Hacker
Reviving just to crash and burn \\ <><
---------> Why can't you listen as love screams everywhere? <--------


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

\
 
 \ /
  Last update: 2005-03-22 13:42    [W:0.046 / U:0.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site