diff options
author | Hugh Dickins <hugh.dickins@tiscali.co.uk> | 2009-08-24 11:30:28 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2009-08-24 15:53:01 -0400 |
commit | 353d5c30c666580347515da609dd74a2b8e9b828 (patch) | |
tree | 03cf3b5c0bc2ce08a12af303b141503ad833178f | |
parent | 0257a0c0c1997aac28420e784b3ef8f3ce17f093 (diff) |
mm: fix hugetlb bug due to user_shm_unlock call
2.6.30's 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 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.
Hugh changed Stefan's suggested patch: can_do_hugetlb_shm() at the
time of shm_destroy() may give a different answer from at the time
of hugetlb_file_setup(). And fixed newseg()'s no_id error path,
which has missed user_shm_unlock() ever since it came in 2.6.9.
Reported-by: Stefan Huber <shuber2@gmail.com>
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Tested-by: Stefan Huber <shuber2@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | fs/hugetlbfs/inode.c | 20 | ||||
-rw-r--r-- | include/linux/hugetlb.h | 6 | ||||
-rw-r--r-- | ipc/shm.c | 8 |
3 files changed, 21 insertions, 13 deletions
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index 941c8425c10b..cb88dac8ccaa 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c | |||
@@ -935,26 +935,28 @@ static int can_do_hugetlb_shm(void) | |||
935 | return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group); | 935 | return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group); |
936 | } | 936 | } |
937 | 937 | ||
938 | struct file *hugetlb_file_setup(const char *name, size_t size, int acctflag) | 938 | struct file *hugetlb_file_setup(const char *name, size_t size, int acctflag, |
939 | struct user_struct **user) | ||
939 | { | 940 | { |
940 | int error = -ENOMEM; | 941 | int error = -ENOMEM; |
941 | int unlock_shm = 0; | ||
942 | struct file *file; | 942 | struct file *file; |
943 | struct inode *inode; | 943 | struct inode *inode; |
944 | struct dentry *dentry, *root; | 944 | struct dentry *dentry, *root; |
945 | struct qstr quick_string; | 945 | struct qstr quick_string; |
946 | struct user_struct *user = current_user(); | ||
947 | 946 | ||
947 | *user = NULL; | ||
948 | if (!hugetlbfs_vfsmount) | 948 | if (!hugetlbfs_vfsmount) |
949 | return ERR_PTR(-ENOENT); | 949 | return ERR_PTR(-ENOENT); |
950 | 950 | ||
951 | if (!can_do_hugetlb_shm()) { | 951 | if (!can_do_hugetlb_shm()) { |
952 | if (user_shm_lock(size, user)) { | 952 | *user = current_user(); |
953 | unlock_shm = 1; | 953 | if (user_shm_lock(size, *user)) { |
954 | WARN_ONCE(1, | 954 | WARN_ONCE(1, |
955 | "Using mlock ulimits for SHM_HUGETLB deprecated\n"); | 955 | "Using mlock ulimits for SHM_HUGETLB deprecated\n"); |
956 | } else | 956 | } else { |
957 | *user = NULL; | ||
957 | return ERR_PTR(-EPERM); | 958 | return ERR_PTR(-EPERM); |
959 | } | ||
958 | } | 960 | } |
959 | 961 | ||
960 | root = hugetlbfs_vfsmount->mnt_root; | 962 | root = hugetlbfs_vfsmount->mnt_root; |
@@ -996,8 +998,10 @@ out_inode: | |||
996 | out_dentry: | 998 | out_dentry: |
997 | dput(dentry); | 999 | dput(dentry); |
998 | out_shm_unlock: | 1000 | out_shm_unlock: |
999 | if (unlock_shm) | 1001 | if (*user) { |
1000 | user_shm_unlock(size, user); | 1002 | user_shm_unlock(size, *user); |
1003 | *user = NULL; | ||
1004 | } | ||
1001 | return ERR_PTR(error); | 1005 | return ERR_PTR(error); |
1002 | } | 1006 | } |
1003 | 1007 | ||
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 2723513a5651..5cbc620bdfe0 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h | |||
@@ -10,6 +10,7 @@ | |||
10 | #include <asm/tlbflush.h> | 10 | #include <asm/tlbflush.h> |
11 | 11 | ||
12 | struct ctl_table; | 12 | struct ctl_table; |
13 | struct user_struct; | ||
13 | 14 | ||
14 | int PageHuge(struct page *page); | 15 | int PageHuge(struct page *page); |
15 | 16 | ||
@@ -146,7 +147,8 @@ static inline struct hugetlbfs_sb_info *HUGETLBFS_SB(struct super_block *sb) | |||
146 | 147 | ||
147 | extern const struct file_operations hugetlbfs_file_operations; | 148 | extern const struct file_operations hugetlbfs_file_operations; |
148 | extern struct vm_operations_struct hugetlb_vm_ops; | 149 | extern struct vm_operations_struct hugetlb_vm_ops; |
149 | struct file *hugetlb_file_setup(const char *name, size_t, int); | 150 | struct file *hugetlb_file_setup(const char *name, size_t size, int acct, |
151 | struct user_struct **user); | ||
150 | int hugetlb_get_quota(struct address_space *mapping, long delta); | 152 | int hugetlb_get_quota(struct address_space *mapping, long delta); |
151 | void hugetlb_put_quota(struct address_space *mapping, long delta); | 153 | void hugetlb_put_quota(struct address_space *mapping, long delta); |
152 | 154 | ||
@@ -168,7 +170,7 @@ static inline void set_file_hugepages(struct file *file) | |||
168 | 170 | ||
169 | #define is_file_hugepages(file) 0 | 171 | #define is_file_hugepages(file) 0 |
170 | #define set_file_hugepages(file) BUG() | 172 | #define set_file_hugepages(file) BUG() |
171 | #define hugetlb_file_setup(name,size,acctflag) ERR_PTR(-ENOSYS) | 173 | #define hugetlb_file_setup(name,size,acct,user) ERR_PTR(-ENOSYS) |
172 | 174 | ||
173 | #endif /* !CONFIG_HUGETLBFS */ | 175 | #endif /* !CONFIG_HUGETLBFS */ |
174 | 176 | ||
@@ -174,7 +174,7 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp) | |||
174 | shm_unlock(shp); | 174 | shm_unlock(shp); |
175 | if (!is_file_hugepages(shp->shm_file)) | 175 | if (!is_file_hugepages(shp->shm_file)) |
176 | shmem_lock(shp->shm_file, 0, shp->mlock_user); | 176 | shmem_lock(shp->shm_file, 0, shp->mlock_user); |
177 | else | 177 | else if (shp->mlock_user) |
178 | user_shm_unlock(shp->shm_file->f_path.dentry->d_inode->i_size, | 178 | user_shm_unlock(shp->shm_file->f_path.dentry->d_inode->i_size, |
179 | shp->mlock_user); | 179 | shp->mlock_user); |
180 | fput (shp->shm_file); | 180 | fput (shp->shm_file); |
@@ -369,8 +369,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params) | |||
369 | /* hugetlb_file_setup applies strict accounting */ | 369 | /* hugetlb_file_setup applies strict accounting */ |
370 | if (shmflg & SHM_NORESERVE) | 370 | if (shmflg & SHM_NORESERVE) |
371 | acctflag = VM_NORESERVE; | 371 | acctflag = VM_NORESERVE; |
372 | file = hugetlb_file_setup(name, size, acctflag); | 372 | file = hugetlb_file_setup(name, size, acctflag, |
373 | shp->mlock_user = current_user(); | 373 | &shp->mlock_user); |
374 | } else { | 374 | } else { |
375 | /* | 375 | /* |
376 | * Do not allow no accounting for OVERCOMMIT_NEVER, even | 376 | * Do not allow no accounting for OVERCOMMIT_NEVER, even |
@@ -410,6 +410,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params) | |||
410 | return error; | 410 | return error; |
411 | 411 | ||
412 | no_id: | 412 | no_id: |
413 | if (shp->mlock_user) /* shmflg & SHM_HUGETLB case */ | ||
414 | user_shm_unlock(size, shp->mlock_user); | ||
413 | fput(file); | 415 | fput(file); |
414 | no_file: | 416 | no_file: |
415 | security_shm_free(shp); | 417 | security_shm_free(shp); |