Re: harden against corrupt symlinks

[Andreas Dilger <adilger@clusterfs.com>]

On May 31, 2005  16:02 -0700, Mike Waychison wrote:
> We've hit the situation a few times where a corrupt symlink could easily 
> oops the kernel.  The problem was tracked down to an older e2fsutils 
> that didn't do much sanity checking on symlinks during a fsck.  This 
> patch uses strnlen when reading in the symlink and ensures that it 
> doesn't exceed PATH_MAX.
> 
> Would you accept this kind of 'hardening'?
> 
> Signed-off-by: Mike Waychison <mikew@google.com>

Andrew Morton suggested a very similar fix 3 years ago to this list:

Subject: Re: ext3 -> crash -> fsck -> readlink -> oops

I thought it made it into the kernel then, but I guess not.

> --- linux-2.6/fs/namei.c	2005-05-24 11:13:09.000000000 -0700
> +++ linux-2.6/fs/namei.c	2005-05-24 11:13:12.000000000 -0700
> @@ -1936,7 +1936,12 @@ int vfs_readlink(struct dentry *dentry, 
>  	if (IS_ERR(link))
>  		goto out;
>  
> -	len = strlen(link);
> +	len = strnlen(link, PATH_MAX);
> +	if (len == PATH_MAX) {
> +		len = -ENAMETOOLONG;
> +		goto out;
> +	}
> +
>  	if (len > (unsigned) buflen)
>  		len = buflen;
>  	if (copy_to_user(buffer, link, len))
> @@ -1953,6 +1958,11 @@ __vfs_follow_link(struct nameidata *nd, 
>  	if (IS_ERR(link))
>  		goto fail;
>  
> +	if (strnlen(link, PATH_MAX) == PATH_MAX) {
> +		link = ERR_PTR(-ENAMETOOLONG);
> +		goto fail;
> +	}
> +
>  	if (*link == '/') {
>  		path_release(nd);
>  		if (!walk_init_root(link, nd))


Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.