Re: PATCH: smart symlink loop detection.

H. Peter Anvin (hpa@transmeta.com)
20 Apr 1998 19:07:35 GMT


Followup to: <Pine.GSO.3.96.980420113705.27291B-100000@csb>
By author: "Adam D. Bradley" <artdodge@cs.bu.edu>
In newsgroup: linux.dev.kernel

> Ok, I spent the weekend kludging around with ways of doing flat
> symlink resolution.
>
> I took a look at how FreeBSD does it... apparently, when in their
> equivalent of lookup_dentry(), when they discover a symlink component
> in the pathname they overwrite it in-place with the symlink's "value",
> then re-start the resolution process using the new name (w/o changing
> "base"). If expanding a symlink would cause the buffer to overflow,
> they return -ENAMETOOLONG. Simple, graceful, makes sense, and allows
> "max-symlinks" to be implemented as a simple, flat counter/countdown.
> So I coded up a new version of lookup_dentry() that does this
> insatead, and it seems to work most of the time.

There is quite a bit to be said for this approach.

> There's just one problem: I need to call inode->i_op->readlink() to
> get the "value" of a symlink. ext2_readlink (among others I suspect)
> uses copy_to_user(), which _sometimes_ returns -EFAULT when trying to
> write the symlink's text to the buffers I'm using. (This happens w/
> all sorts of buffers: static char[]'s in the fn, kmalloc(PATH_MAX+1,
> GFP_{KERNEL,USER}), and get_free_page(GFP_{KERNEL,USER}).) My guess
> is it doesn't like writing to kernel-space buffers. BSD handles this
> using a flag in their "uio" structure. I'm sure this can be done in
> Linux, there's just something obvious I'm missing.

Another problem: it's wrong; readlink() and follow_link() are
*different* operations under Linux. You need to use follow_link(),
and if the interface of follow_link() doesn't match what you want then
you have to change the VFS.

> Another nit-pick - since there's no way for me to know (w/o using
> fs-specific code) the length of a symlink's value until I've copied it
> (return of i_op->readlink), I need to use two buffers. If there were
> a way to know beforehand, I could dispense with that second buffer (a
> second page of memory) and do everything in-place. This would,
> however, probably require a new i_op.

You need that anyway. See above. Also, be *very* aware of the
potential for race conditions here.

-hpa

-- 
    PGP: 2047/2A960705 BA 03 D3 2C 14 A8 A8 BD  1E DF FE 69 EE 35 BD 74
    See http://www.zytor.com/~hpa/ for web page and full PGP public key
        I am Bahá'í -- ask me about it or see http://www.bahai.org/
   "To love another person is to see the face of God." -- Les Misérables

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