[PATCH] fs: Check fsname's length in get_filesystem_list().

[PATCH] fs: Check fsname's length in get_filesystem_list().

[Tetsuo Handa]

Since "struct file_system_type"->name does not limit the max length,
one might pass a name with strlen(name) >= 80.

  while (tmp && len < PAGE_SIZE - 80) {
          len += sprintf(buf+len, "%s\t%s\n",
                  (tmp->fs_flags & FS_REQUIRES_DEV) ? "" : "nodev",
                  tmp->name);

This can cause buffer overrun if somebody builts in very very long filesystem
name (as shown in example code below); although unlikely happens, for
get_filesystem_list() is called only once upon boot.

----- dummyfs.c start : Build with obj-y += dummyfs.o -----
#include <linux/fs.h>
#include <linux/init.h>

static char name[PAGE_SIZE * 16];

static int dummy_get_sb(struct file_system_type *fs_type,
			int flags, const char *dev_name, void *data,
			struct vfsmount *mnt)
{
	return -ENOMEM;
}

static struct file_system_type dummy_fs_type = {
	.name       = name,
	.get_sb     = dummy_get_sb,
	.kill_sb    = kill_litter_super,
};

static int __init dummyfs_init(void)
{
	memset(name, 'X', sizeof(name));
	name[sizeof(name) - 1] = '\0';
	return register_filesystem(&dummy_fs_type);
}

__initcall(dummyfs_init);
----- dummyfs.c end -----

----------
[PATCH] fs: Check fsname's length in get_filesystem_list().

Since "struct file_system_type"->name does not limit the max length,
one can pass a name with strlen(name) >= 80.

This can cause buffer overrun; although unlikely happens, for
get_filesystem_list() is called only once upon boot.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/filesystems.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

--- linux-2.6.30-rc1.orig/fs/filesystems.c
+++ linux-2.6.30-rc1/fs/filesystems.c
@@ -202,15 +202,17 @@ SYSCALL_DEFINE3(sysfs, int, option, unsi
 int __init get_filesystem_list(char *buf)
 {
 	int len = 0;
-	struct file_system_type * tmp;
+	struct file_system_type *tmp;
 
 	read_lock(&file_systems_lock);
-	tmp = file_systems;
-	while (tmp && len < PAGE_SIZE - 80) {
-		len += sprintf(buf+len, "%s\t%s\n",
-			(tmp->fs_flags & FS_REQUIRES_DEV) ? "" : "nodev",
-			tmp->name);
-		tmp = tmp->next;
+	for (tmp = file_systems; tmp; tmp = tmp->next) {
+		/* 8 is for "nodev" + '\t' + '\n' + '\0'. */
+		const int n = strlen(tmp->name) + 8;
+		if (len + n >= PAGE_SIZE)
+			break;
+		len += sprintf(buf + len, "%s\t%s\n",
+			       (tmp->fs_flags & FS_REQUIRES_DEV) ? "" : "nodev",
+			       tmp->name);
 	}
 	read_unlock(&file_systems_lock);
 	return len;

Re: [PATCH] fs: Check fsname's length in get_filesystem_list().

[Al Viro]

On Thu, Apr 09, 2009 at 08:18:11PM +0900, Tetsuo Handa wrote:
> Since "struct file_system_type"->name does not limit the max length,
> one might pass a name with strlen(name) >= 80.
> 
>   while (tmp && len < PAGE_SIZE - 80) {
>           len += sprintf(buf+len, "%s\t%s\n",
>                   (tmp->fs_flags & FS_REQUIRES_DEV) ? "" : "nodev",
>                   tmp->name);
> 
> This can cause buffer overrun if somebody builts in very very long filesystem
> name (as shown in example code below); although unlikely happens, for
> get_filesystem_list() is called only once upon boot.

Doctor, it hurts when I do it...