Skip to contentSkip to conversation

2.6.22-rc6-mm1: hugetlbfs handle empty options string

PATCH In review Lee Schermerhorn 2 participants 2 messages Jul 10, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 2.6.22-rc6-mm1: hugetlbfs handle empty options string

[PATCH] 2.6.22-rc6-mm1 - hugetlbfs handle empty options string

I was seeing a null pointer deref in fs/super.c:vfs_kern_mount(). Some file system get_sb() handler was returning NULL mnt_sb with a non-negative return value. I also noticed a "hugetlbfs: Bad mount option:" message in the log.

Turns out that hugetlbfs_parse_options() was not checking for an empty option string after call to strsep(). On failure, hugetlbfs_parse_options() returns 1. hugetlbfs_fill_super() just passed this return code back up the call stack where vfs_kern_mount() missed the error and proceeded with a NULL mnt_sb.

Apparently introduced by patch:
hugetlbfs-use-lib-parser-fix-docs.patch

The problem was exposed by this line in my fstab:

none /huge hugetlbfs defaults 0 0

It can also be demonstrated by invoking mount of hugetlbfs directly with no options or a bogus option.

This patch:

1) adds the check for empty option to hugetlbfs_parse_options(),
2) enhances the error message to bracket any unrecognized option with quotes ,
3) modifies hugetlbfs_parse_options() to return -EINVAL on any unrecognized option,
4) adds a BUG_ON() to vfs_kern_mount() to catch any get_sb() handler that returns a NULL mnt->mnt_sb with a return value

Quoted 1 line

= 0.

 fs/hugetlbfs/inode.c |    8 +++++---
 fs/super.c           |    1 +
 2 files changed, 6 insertions(+), 3 deletions(-)
fs/hugetlbfs/inode.c +5 -3
Index: Linux/fs/hugetlbfs/inode.c
===================================================================
@@ -625,6 +625,8 @@ hugetlbfs_parse_options(char *options, s
 
 	while ((p = strsep(&options, ",")) != NULL) {
 		int token;
+		if (!*p)
+			continue;
 
 		token = match_token(p, tokens, args);
 		switch (token) {
@@ -669,8 +671,9 @@ hugetlbfs_parse_options(char *options, s
 			break;
 
 		default:
-			printk(KERN_ERR "hugetlbfs: Bad mount option: %s\n", p);
- 			return 1;
+			printk(KERN_ERR "hugetlbfs: Bad mount option: \"%s\"\n",
+				 p);
+			return -EINVAL;
 			break;
 		}
 	}
@@ -697,7 +700,6 @@ hugetlbfs_fill_super(struct super_block 
 	config.gid = current->fsgid;
 	config.mode = 0755;
 	ret = hugetlbfs_parse_options(data, &config);
-
 	if (ret)
 		return ret;
 
fs/super.c +1 -0
Index: Linux/fs/super.c
===================================================================
@@ -880,6 +880,7 @@ vfs_kern_mount(struct file_system_type *
 	error = type->get_sb(type, flags, name, data, mnt);
 	if (error < 0)
 		goto out_free_secdata;
+	BUG_ON(!mnt->mnt_sb);
 
  	error = security_sb_kern_mount(mnt->mnt_sb, secdata);
  	if (error)
Acked-by on v1 1/1
#1 lore raw
Quoted 32 lines · On Tue, 10 Jul 2007 15:59:00 -0400 Lee Schermerhorn wrote:

[PATCH] 2.6.22-rc6-mm1 - hugetlbfs handle empty options string

I was seeing a null pointer deref in fs/super.c:vfs_kern_mount(). Some file system get_sb() handler was returning NULL mnt_sb with a non-negative return value. I also noticed a "hugetlbfs: Bad mount option:" message in the log.

Turns out that hugetlbfs_parse_options() was not checking for an empty option string after call to strsep(). On failure, hugetlbfs_parse_options() returns 1. hugetlbfs_fill_super() just passed this return code back up the call stack where vfs_kern_mount() missed the error and proceeded with a NULL mnt_sb.

Apparently introduced by patch:
hugetlbfs-use-lib-parser-fix-docs.patch

The problem was exposed by this line in my fstab:

none /huge hugetlbfs defaults 0 0

It can also be demonstrated by invoking mount of hugetlbfs directly with no options or a bogus option.

This patch:

1) adds the check for empty option to hugetlbfs_parse_options(),
2) enhances the error message to bracket any unrecognized option with quotes ,
3) modifies hugetlbfs_parse_options() to return -EINVAL on any unrecognized option,
4) adds a BUG_ON() to vfs_kern_mount() to catch any get_sb() handler that returns a NULL mnt->mnt_sb with a return value

Quoted 7 lines
Quoted 1 line

= 0.

 fs/hugetlbfs/inode.c |    8 +++++---
 fs/super.c           |    1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

Argh. Thanks.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***