Re: generic_file_llseek() broken?

[Andreas Dilger]

On Nov 15, 2001  00:08 +0000, Alan Cox wrote:
> > I was recently testing a bit with creating very large files on ext2/ext3
> > (just to see if limits were what they should be).  Now, I know that ext2/3
> > allows files just shy of 2TB right now, because of an issue with i_blocks
> > being in units of 512-byte sectors, instead of fs blocks.
> 
> Does 2.4.13-ac7 show the same. There were some off by one fixes and its
> possible I managed to forget to feed Linus one

The test was done on 2.4.13, but I looked through 2.4.14, 2.4.15-pre4, and
2.4.13-ac8, and the code in question was not touched.  It is definitely not
just an off-by-one error, since if I try to create a 40TB file it is the same
problem (i.e. llseek returns -EINVAL, dd tries to read 40TB of file to try
and make the file offset correct).  When it would eventually get to the
correct offset (I can't see anything on the read path checking s_maxbytes,
and don't know what LFS says on this) it would just get EFBIG as soon as
it tries to write anything.

An example of what I'm thinking should be changed is (not tested, compiled,
anything yet; EFBIG might be EOVERFLOW, I don't know) just for illustration:

--- linux.orig/fs/read_write.c	Tue Aug 14 12:09:09 2001
+++ linux/fs/read_write.c	Wed Nov 14 16:11:23 2001
@@ -36,15 +36,24 @@
 		case 1:
 			offset += file->f_pos;
 	}
-	retval = -EINVAL;
-	if (offset>=0 && offset<=file->f_dentry->d_inode->i_sb->s_maxbytes) {
-		if (offset != file->f_pos) {
-			file->f_pos = offset;
-			file->f_reada = 0;
-			file->f_version = ++event;
-		}
-		retval = offset;
+
+	if (offset < 0) {
+		retval = -EINVAL;
+		goto out;
 	}
+
+	if (offset > file->f_dentry->d_inode->i_sb->s_maxbytes) {
+		retval = -EFBIG;
+		goto out;
+	}
+
+	if (offset != file->f_pos) {
+		file->f_pos = offset;
+		file->f_reada = 0;
+		file->f_version = ++event;
+	}
+	retval = offset;
+out:
 	return retval;
 }

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