hugetlbfs: fix error path in module_init

init_hugetlbfs_fs() needs to unregister hugetlbfs
when kern_mount() returns error.

---
 fs/hugetlbfs/inode.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Index: 2.6-mm/fs/hugetlbfs/inode.c
===================================================================
@@ -804,20 +804,23 @@ static int __init init_hugetlbfs_fs(void
 
 	error = register_filesystem(&hugetlbfs_fs_type);
 	if (error)
-		goto out;
+		goto out_cache;
 
 	vfsmount = kern_mount(&hugetlbfs_fs_type);
 
-	if (!IS_ERR(vfsmount)) {
-		hugetlbfs_vfsmount = vfsmount;
-		return 0;
+	if (IS_ERR(vfsmount)) {
+		error = PTR_ERR(vfsmount);
+		goto out_fs;
 	}
+	hugetlbfs_vfsmount = vfsmount;
 
-	error = PTR_ERR(vfsmount);
+	return 0;
+
+out_fs:
+	unregister_filesystem(&hugetlbfs_fs_type);
+out_cache:
+	kmem_cache_destroy(hugetlbfs_inode_cachep);
 
- out:
-	if (error)
-		kmem_cache_destroy(hugetlbfs_inode_cachep);
 	return error;
 }
 

HUGETLBFS is bool, so __init function should panic more and __exit function should be removed. Or someone is planning making it tristate?

Seems reasonable to make it panic. Because hugetlbfs_vfsmount is not initialized when error happen, and shmget() (ipc/shm.c) can cause NULL pointer dereference by hugetlb_zero_setup().

Or we can add NULL check for hugetlbfs_vfsmount in hugetlbfs_vfsmount().

It'd be nice for it to be normal enough to go tristate, but there isn't going to be any effort expended directly to that end. Might as well remove the exit function. Probably mostly pointless to try to avoid panicking, but I'd not bother with new panics as a simplification just because panicking is somewhat rude.

- wli

This patch resolves a clear failure to error check.

I'll work out how to do anything with git and point people at a tree shortly.

-- wli

I reconsider this patch. It was wrong. Because it is overkill to unregister filesystem just for hugetlbfs_vfsmount mount failure. Because hugetlbfs_vfsmount is only used for shumget() system call and also there is some race condition between register_filesystem() and unregister_filesystem(). (it's not likely happen though)

So what we can do for hugetlbfs module_init failure is adding NULL check for hugetlbfs_vfsmount in hugetlb_zero_setup().

If hugetlbfs module_init() fails, hugetlbfs_vfsmount is not initialized and shmget() with SHM_HUGETLB flag will cause NULL pointer dereference.

Index: 2.6-mm/fs/hugetlbfs/inode.c
===================================================================
@@ -740,6 +740,9 @@ struct file *hugetlb_zero_setup(size_t s
 	char buf[16];
 	static atomic_t counter;
 
+	if (!hugetlbfs_vfsmount)
+		return ERR_PTR(-ENOENT);
+
 	if (!can_do_hugetlb_shm())
 		return ERR_PTR(-EPERM);