[PATCH] detecting overflows in nfs_statfs

[PATCH] detecting overflows in nfs_statfs

[Steve Dickson]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

Hello,

Here is 2.4  patch that allows nfs_statfs() to detect values that are
too large to deal with (i.e. > 32bit on a 32bit machine).  So instead
of returning garbage (as it does today), it returns all -1 which
commands (like df) know how interpret. For example:
df without the patch

Filesystem           1K-blocks      Used Available Use% Mounted on
pdl585-1:/           7108754432 -147573952589283246080 33966574080 101% 
/mnt/dl585-1

df with the patch:
Filesystem           1K-blocks      Used Available Use% Mounted on
pdl585-1:/                   1         1         1   0% /mnt/dl585-1

Which should be fairly obvious that something went wrong.....

Comments?

Maybe this issue has already been address on the list (I know Olaf
recently sent out a 2.6 patch) but if it hasn't.... maybe this is something
Marcelo would be interested in?

SteveD.


[-- Attachment #2: linux-2.4.21-nfs-eoverflow.patch --]
[-- Type: text/plain, Size: 1620 bytes --]

--- linux-2.4.21/fs/nfs/inode.c.orig	2004-06-09 12:52:29.000000000 -0400
+++ linux-2.4.21/fs/nfs/inode.c	2004-06-09 12:18:28.000000000 -0400
@@ -593,6 +593,7 @@ out_fail:
 	return NULL;
 }
 
+#define TOOBIG(_arg) ((_arg) > ~0UL)
 static int
 nfs_statfs(struct super_block *sb, struct statfs *buf)
 {
@@ -605,12 +606,27 @@ nfs_statfs(struct super_block *sb, struc
 
 	error = server->rpc_ops->statfs(server, NFS_FH(sb->s_root->d_inode), &res);
 	buf->f_type = NFS_SUPER_MAGIC;
-	if (error < 0)
+	if (error < 0) {
+		printk("nfs_statfs: statfs error = %d\n", -error);
 		goto out_err;
+	}
 
 	buf->f_bsize = sb->s_blocksize;
 	blockbits = sb->s_blocksize_bits;
 	blockres = (1 << blockbits) - 1;
+
+	/*
+	 * Make sure things fit
+	 */
+	if (TOOBIG(((res.tbytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(((res.fbytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(((res.abytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(res.tfiles) || TOOBIG(res.afiles))
+		goto too_big;
+
 	buf->f_blocks = (res.tbytes + blockres) >> blockbits;
 	buf->f_bfree = (res.fbytes + blockres) >> blockbits;
 	buf->f_bavail = (res.abytes + blockres) >> blockbits;
@@ -618,11 +634,15 @@ nfs_statfs(struct super_block *sb, struc
 	buf->f_ffree = res.afiles;
 	buf->f_namelen = server->namelen;
 	return 0;
+
+ too_big:
+ 	dprintk("nfs_statfs: failed: EOVERFLOW\n");
+
  out_err:
-	printk("nfs_statfs: statfs error = %d\n", -error);
 	buf->f_bsize = buf->f_blocks = buf->f_bfree = buf->f_bavail = -1;
 	return 0;
 }
+#undef TOOBIG
 
 static int nfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 {

Re: [PATCH] detecting overflows in nfs_statfs

[Steve Dickson]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

Steve Dickson wrote:

> Here is 2.4  patch that allows nfs_statfs() to detect values that are
> too large to deal with (i.e. > 32bit on a 32bit machine).  So instead
> of returning garbage (as it does today), it returns all -1 which
> commands (like df) know how interpret. For example:
> df without the patch
>
> Filesystem           1K-blocks      Used Available Use% Mounted on
> pdl585-1:/           7108754432 -147573952589283246080 33966574080 
> 101% /mnt/dl585-1
>
> df with the patch:
> Filesystem           1K-blocks      Used Available Use% Mounted on
> pdl585-1:/                   1         1         1   0% /mnt/dl585-1
>
> Which should be fairly obvious that something went wrong.....
>
> Comments?
>
> Maybe this issue has already been address on the list (I know Olaf
> recently sent out a 2.6 patch) but if it hasn't.... maybe this is 
> something
> Marcelo would be interested in?
>
This is an update to the previous patch that 1) always sets f_namelen
to a valid value since apps could be depending on it and 2) f_files
and f_ffree are set to -1 on any an overflows.

SteveD.



[-- Attachment #2: linux-2.4.21-nfs-eoverflow4.patch --]
[-- Type: text/plain, Size: 1662 bytes --]

--- linux-2.4.21/fs/nfs/inode.c.orig	2004-06-09 12:52:29.000000000 -0400
+++ linux-2.4.21/fs/nfs/inode.c	2004-06-16 21:14:37.000000000 -0400
@@ -593,6 +593,7 @@ out_fail:
 	return NULL;
 }
 
+#define TOOBIG(_arg) ((_arg) > LONG_MAX)
 static int
 nfs_statfs(struct super_block *sb, struct statfs *buf)
 {
@@ -605,24 +606,44 @@ nfs_statfs(struct super_block *sb, struc
 
 	error = server->rpc_ops->statfs(server, NFS_FH(sb->s_root->d_inode), &res);
 	buf->f_type = NFS_SUPER_MAGIC;
-	if (error < 0)
+	if (error < 0) {
+		printk("nfs_statfs: statfs error = %d\n", -error);
 		goto out_err;
+	}
 
 	buf->f_bsize = sb->s_blocksize;
 	blockbits = sb->s_blocksize_bits;
 	blockres = (1 << blockbits) - 1;
+	buf->f_namelen = server->namelen;
+
+	/*
+	 * Make sure things fit
+	 */
+	if (TOOBIG(((res.tbytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(((res.fbytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(((res.abytes + blockres) >> blockbits)))
+		goto too_big;
+	if (TOOBIG(res.tfiles) || TOOBIG(res.afiles))
+		goto too_big;
+
 	buf->f_blocks = (res.tbytes + blockres) >> blockbits;
 	buf->f_bfree = (res.fbytes + blockres) >> blockbits;
 	buf->f_bavail = (res.abytes + blockres) >> blockbits;
 	buf->f_files = res.tfiles;
 	buf->f_ffree = res.afiles;
-	buf->f_namelen = server->namelen;
 	return 0;
+
+ too_big:
+ 	dprintk("nfs_statfs: failed: EOVERFLOW\n");
+	buf->f_files = buf->f_ffree = -1;
+
  out_err:
-	printk("nfs_statfs: statfs error = %d\n", -error);
 	buf->f_bsize = buf->f_blocks = buf->f_bfree = buf->f_bavail = -1;
 	return 0;
 }
+#undef TOOBIG
 
 static int nfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 {