On Fri, 2006-08-18 at 00:27 -0600, Andreas Dilger wrote:
> On Aug 11, 2006 15:48 -0700, Andrew Morton wrote:
> >
> > Given that this is the VFS correcting for filesystem misbehaviour, it would
> > seem to make sense to perform this correction at a low level, immediately
> > after the filesytem driver has returned us the pathname component. An
> > apparently-obvious way of doing this is to wrap ->follow_link. Can anyone
> > think of a smarter way?
>
> #define PATH_MAX 4096 /* # chars in a path name including nul */
>
> One possibility is for the case of long symlinks to always set the last byte
> of the page to NUL to ensure that the link is terminated. This appears
> easily doable by having page_getlink() do the NUL termination after kmap()
> but before returning, something like:
>
> static char *page_getlink(struct dentry * dentry, struct page **ppage)
> {
> char *link;
> page = read_cache_page(mapping, 0, (filler_t *)mapping->a_ops->readpage,
> NULL);
> if (IS_ERR(page))
> goto sync_fail;
> wait_on_page_locked(page);
> if (!PageUptodate(page))
> goto async_fail;
> *ppage = page;
> - return kmap(page);
> + link = kmap(page);
> + /* PATH_MAX is strictly <= PAGE_SIZE */
> + link[PATH_MAX - 1] = '\0';
> + return link;
This seems reasonable.
>
>
> Many of the other filesystems that don't use page_follow_link_light()
> already do NUL termination themselves.
>
> nfs_follow_link() is very similar, but not identical and needs the same fix.
jfs_follow_link() too. :-)
> ocfs2_page_getlink() is an exact duplicate of page_getlink() and the bug
> duplication could be avoided if page_getlink() was exported. Otherwise it
> needs the same fix.
>
>
> I _think_ PATH_MAX is the right thing here (instead of PAGE_SIZE), since
> the caller expects at most PATH_MAX in the returned link, and PAGE_SIZE
> may be considerably larger. I don't think PATH_MAX will ever be larger
> than PAGE_SIZE. We could also use min(PAGE_SIZE, PATH_MAX) and it would
> be resolved at compile time, but it seems wishy-washy to me.
>
> The other option is to actually get the link length out of the filesystem
> itself, and avoid strlen(link) entirely, but that is a more complex change.
Is it? Is there any file system where the link length is not i_size?
--
David Kleikamp
IBM Linux Technology Center
-
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
