ext3 -> crash -> fsck -> readlink -> oops

ext3 -> crash -> fsck -> readlink -> oops

[H. Peter Anvin]

Hi there,

We just suffered some SCSI bus problems on kernel.org that resulted in
significant data loss on our mirror volume, but not enough to ditch
the whole filesystem and start over (rsync --checksum is a good
thing.)  However, we keep getting oopses, ostensibly as the result of
a garbaged symlink.  This filesystem has been run through fsck -y
(e2fstools 1.26).

Oops details at:

    http://userweb.kernel.org/~hpa/oops-20020516/

The kernel running is 2.4.19-pre8.

	-hpa

-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<amsp@zytor.com>

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

[Andrew Morton]

"H. Peter Anvin" wrote:
> 
> Hi there,
> 
> We just suffered some SCSI bus problems on kernel.org that resulted in
> significant data loss on our mirror volume, but not enough to ditch
> the whole filesystem and start over (rsync --checksum is a good
> thing.)  However, we keep getting oopses, ostensibly as the result of
> a garbaged symlink.  This filesystem has been run through fsck -y
> (e2fstools 1.26).
> 
> Oops details at:
> 
>     http://userweb.kernel.org/~hpa/oops-20020516/
> 
> The kernel running is 2.4.19-pre8.
> 

I assume that was `fsck -fy'?

You seem have a 4k symlink.   I'd have expected fsck to detect that.

It crashed because vfs_readlink ran off the end of the page.

--- linux-2.4.19-pre8/fs/namei.c	Thu May  2 17:44:56 2002
+++ linux-akpm/fs/namei.c	Thu May 16 19:34:57 2002
@@ -1947,7 +1947,7 @@ int vfs_readlink(struct dentry *dentry, 
 	if (IS_ERR(link))
 		goto out;
 
-	len = strlen(link);
+	len = strnlen(link, PAGE_CACHE_SIZE);
 	if (len > (unsigned) buflen)
 		len = buflen;
 	if (copy_to_user(buffer, link, len))


-

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

[H. Peter Anvin]

Andrew Morton wrote:
>> 
> I assume that was `fsck -fy'?
> 
> You seem have a 4k symlink.   I'd have expected fsck to detect that.
> 
> It crashed because vfs_readlink ran off the end of the page.
> 

Either that... or someone is relying on magic null termination that 
isn't provided by fsck.  Either way, the kernel shouldn't oops... 
ext3_panic() perhaps, but not oops...

	-hpa

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

[Andreas Dilger]

On May 16, 2002  19:37 -0700, Andrew Morton wrote:
> You seem have a 4k symlink.   I'd have expected fsck to detect that.

I just took a look (built a simple testcase) and it does not detect
string overflows for long symlinks.  It does for fast symlinks, because
the data is available right away when it is checking the inode data,
but the block data isn't validated at that time, nor later in the run.

> It crashed because vfs_readlink ran off the end of the page.
> 
> --- linux-2.4.19-pre8/fs/namei.c	Thu May  2 17:44:56 2002
> +++ linux-akpm/fs/namei.c	Thu May 16 19:34:57 2002
> @@ -1947,7 +1947,7 @@ int vfs_readlink(struct dentry *dentry, 
>  	if (IS_ERR(link))
>  		goto out;
>  
> -	len = strlen(link);
> +	len = strnlen(link, PAGE_CACHE_SIZE);
>  	if (len > (unsigned) buflen)
>  		len = buflen;
>  	if (copy_to_user(buffer, link, len))

This will fix the oops on behalf of the kernel.  I'm just working on a
patch to e2fsck to fix that side of things.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/

Re: [Ext2-devel] Re: ext3 -> crash -> fsck -> readlink -> oops

[Stephen C. Tweedie]

Hi,

On Thu, May 16, 2002 at 07:37:36PM -0700, Andrew Morton wrote:

> -	len = strlen(link);
> +	len = strnlen(link, PAGE_CACHE_SIZE);
>  	if (len > (unsigned) buflen)
>  		len = buflen;

Well, if we're going to be testing against buflen anyway, then

--- fs/namei.c.~1~	Thu May  9 17:13:32 2002
+++ fs/namei.c	Fri May 17 10:37:58 2002
@@ -1947,9 +1947,7 @@
 	if (IS_ERR(link))
 		goto out;
 
-	len = strlen(link);
-	if (len > (unsigned) buflen)
-		len = buflen;
+	len = strnlen(link, buflen);
 	if (copy_to_user(buffer, link, len))
 		len = -EFAULT;
 out:

is cleaner.

--Stephen

Re: [Ext2-devel] Re: ext3 -> crash -> fsck -> readlink -> oops

[Andreas Dilger]

On May 17, 2002  10:40 +0100, Stephen C. Tweedie wrote:
> On Thu, May 16, 2002 at 07:37:36PM -0700, Andrew Morton wrote:
> 
> > -	len = strlen(link);
> > +	len = strnlen(link, PAGE_CACHE_SIZE);
> >  	if (len > (unsigned) buflen)
> >  		len = buflen;
> 
> Well, if we're going to be testing against buflen anyway, then
> 
> --- fs/namei.c.~1~	Thu May  9 17:13:32 2002
> +++ fs/namei.c	Fri May 17 10:37:58 2002
> @@ -1947,9 +1947,7 @@
>  	if (IS_ERR(link))
>  		goto out;
>  
> -	len = strlen(link);
> -	if (len > (unsigned) buflen)
> -		len = buflen;
> +	len = strnlen(link, buflen);
>  	if (copy_to_user(buffer, link, len))
>  		len = -EFAULT;
>  out:

Except that buflen is passed in from userspace, so the user could
allocate a 8MB buffer and try to read a broken symlink to get access
to kernel memory.  How about:

--- fs/namei.c.~1~	Thu May  9 17:13:32 2002
+++ fs/namei.c	Fri May 17 10:37:58 2002
@@ -1947,9 +1947,7 @@
 	if (IS_ERR(link))
 		goto out;
 
-	len = strlen(link);
-	if (len > (unsigned) buflen)
-		len = buflen;
+	len = strnlen(link, min(buflen, PAGE_SIZE));
 	if (copy_to_user(buffer, link, len))
 		len = -EFAULT;
 out:

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/

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

[Andreas Dilger]

On May 16, 2002  19:37 -0700, Andrew Morton wrote:
> "H. Peter Anvin" wrote:
> > 
> > Hi there,
> > 
> > We just suffered some SCSI bus problems on kernel.org that resulted in
> > significant data loss on our mirror volume, but not enough to ditch
> > the whole filesystem and start over (rsync --checksum is a good
> > thing.)  However, we keep getting oopses, ostensibly as the result of
> > a garbaged symlink.  This filesystem has been run through fsck -y
> > (e2fstools 1.26).
> > 
> > Oops details at:
> > 
> >     http://userweb.kernel.org/~hpa/oops-20020516/
> > 
> > The kernel running is 2.4.19-pre8.
> > 
> 
> I assume that was `fsck -fy'?
> 
> You seem have a 4k symlink.   I'd have expected fsck to detect that.
> 
> It crashed because vfs_readlink ran off the end of the page.
> 
> --- linux-2.4.19-pre8/fs/namei.c	Thu May  2 17:44:56 2002
> +++ linux-akpm/fs/namei.c	Thu May 16 19:34:57 2002
> @@ -1947,7 +1947,7 @@ int vfs_readlink(struct dentry *dentry, 
>  	if (IS_ERR(link))
>  		goto out;
>  
> -	len = strlen(link);
> +	len = strnlen(link, PAGE_CACHE_SIZE);
>  	if (len > (unsigned) buflen)
>  		len = buflen;
>  	if (copy_to_user(buffer, link, len))

OK, I posted a patch to e2fsck on ext2-devel (against Ted's BK
repository, but should be OK with 1.27) which should detect and
fix (i.e. remove) the symlinks without nul termination that are
causing this problem.  Let me know how it works (you might want
to try 'e2fsck -fn <dev>' first just to see that it detects the
problem without actually changing the filesystem.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/