Skip to contentSkip to conversation

detecting overflows in nfs_statfs

PATCH In review Steve Dickson 1 participant 2 messages Jun 9, 2004 – Jun 28, 2004

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Steve Dickson

author
#0 lore raw

PATCH 1/1 detecting overflows in nfs_statfs

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.

fs/nfs/inode.c renamed from fs/nfs/inode.c.orig +22 -2
@@ -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)
 {
author
#1 lore raw
Quoted 13 lines · Steve Dickson

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

Quoted 8 lines

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.

fs/nfs/inode.c renamed from fs/nfs/inode.c.orig +24 -3
@@ -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)
 {