[PATCH] mm: fix hugetlb bug due to user_shm_unlock call (fwd)

[James Morris]

---------- Forwarded message ----------
Date: Sun, 23 Aug 2009 11:03:11 +0200
From: Stefan Huber <shuber2@xxxxxxxxx>
To: William Irwin <wli@xxxxxxxxxxxxxx>
Cc: James Morris <jmorris@xxxxxxxxx>, Peter Meerwald <pmeerw@xxxxxxxxxxxxxx>
Subject: [PATCH] mm: fix hugetlb bug due to user_shm_unlock call

This is a multi-part message in MIME format.Dear maintainer!

We have recently detected a kernel oops bug in the hugetlb code. The bug
is still present in the current linux-2.6 git branch (tested until [1]).
We have attached a 'git format-patch'-file that solved problem for us.
The commit message should describe the logic of the bug. Please contact
me if you have further questions or comments.

Sincerely,
Stefan Huber.


[1]  linux/kernel/git/torvalds/linux-2.6.git (commit
     429966b8f644dda2afddb4f834a944e9b46a7645)
From cc3cc9a467680c6c5911fc0e669b3c33f55078a4 Mon Sep 17 00:00:00 2001
From: Stefan Huber <shuber2@xxxxxxxxx>
Date: Fri, 21 Aug 2009 16:18:00 +0200
Subject: [PATCH] mm: fix hugetlb bug due to user_shm_unlock call

The commit 8a0bdec194c21c8fdef840989d0d7b742bb5d4bc removed
user_shm_lock() calls in hugetlb_file_setup() but left the
user_shm_unlock call in shm_destroy().

In detail:
Assume that can_do_hugetlb_shm() returns true and hence user_shm_lock()
is not called in hugetlb_file_setup(). However, user_shm_unlock() is
called in any case in in shm_destroy() and in the following
atomic_dec_and_lock(&up->__count) in free_uid() is executed and if
up->__count gets zero, also cleanup_user_struct() is scheduled.

Note that sched_destroy_user() is empty if CONFIG_USER_SCHED is not set.
However, the ref counter up->__count gets unexpectedly non-positive and
the corresponding structs are freed even though there are live
references to them, resulting in a kernel oops after a lots of
shmget(SHM_HUGETLB)/shmctl(IPC_RMID) cycles and CONFIG_USER_SCHED set.

Reviewed-by: Peter Meerwald <pmeerw@xxxxxxxxxx>
---
 fs/hugetlbfs/inode.c    |    2 +-
 include/linux/hugetlb.h |    2 ++
 ipc/shm.c               |    2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 941c842..8712a58 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -930,7 +930,7 @@ static struct file_system_type hugetlbfs_fs_type = {
 
 static struct vfsmount *hugetlbfs_vfsmount;
 
-static int can_do_hugetlb_shm(void)
+int can_do_hugetlb_shm(void)
 {
 	return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group);
 }
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 2723513..eda7dce 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -146,6 +146,7 @@ static inline struct hugetlbfs_sb_info *HUGETLBFS_SB(struct super_block *sb)
 
 extern const struct file_operations hugetlbfs_file_operations;
 extern struct vm_operations_struct hugetlb_vm_ops;
+int can_do_hugetlb_shm(void);
 struct file *hugetlb_file_setup(const char *name, size_t, int);
 int hugetlb_get_quota(struct address_space *mapping, long delta);
 void hugetlb_put_quota(struct address_space *mapping, long delta);
@@ -168,6 +169,7 @@ static inline void set_file_hugepages(struct file *file)
 
 #define is_file_hugepages(file)			0
 #define set_file_hugepages(file)		BUG()
+#define can_do_hugetlb_shm()			0
 #define hugetlb_file_setup(name,size,acctflag)	ERR_PTR(-ENOSYS)
 
 #endif /* !CONFIG_HUGETLBFS */
diff --git a/ipc/shm.c b/ipc/shm.c
index 15dd238..9e50e6f 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -174,7 +174,7 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
 	shm_unlock(shp);
 	if (!is_file_hugepages(shp->shm_file))
 		shmem_lock(shp->shm_file, 0, shp->mlock_user);
-	else
+	else if(!can_do_hugetlb_shm())
 		user_shm_unlock(shp->shm_file->f_path.dentry->d_inode->i_size,
 						shp->mlock_user);
 	fput (shp->shm_file);
-- 
1.6.3.3