Fix user struct leakage with locked IPC shem segment
PATCH 1/1 Fix user struct leakage with locked IPC shem segment
When user locks an ipc shmem segmant with SHM_LOCK ctl and the
segment is already locked the shmem_lock() function returns 0. After this the subsequent code leaks the existing user struct:
== ipc/shm.c: sys_shmctl() ==
...
err = shmem_lock(shp->shm_file, 1, user);
if (!err) {
shp->shm_perm.mode |= SHM_LOCKED;
shp->mlock_user = user;
}
...==
Other results of this are:
1. the new shp->mlock_user is not get-ed and will point to freed memory when the task dies.
2. the RLIMIT_MEMLOCK is screwed on both user structs.
The exploit looks like this:
==
id = shmget(...);
setresuid(uid, 0, 0);
shmctl(id, SHM_LOCK, NULL);
setresuid(uid + 1, 0, 0);
shmctl(id, SHM_LOCK, NULL);
==
My solution is to return 0 to the userspace and do not change the segment's user.
---
ipc/shm.c renamed from ipc/shm.c.shlfix +0 -0
@@ -715,7 +715,7 @@ asmlinkage long sys_shmctl (int shmid, i
struct user_struct * user = current->user;
if (!is_file_hugepages(shp->shm_file)) {
err = shmem_lock(shp->shm_file, 1, user);
- if (!err) {
+ if (!err && !(shp->shm_perm.mode & SHM_LOCKED)){
shp->shm_perm.mode |= SHM_LOCKED;
shp->mlock_user = user;
}Quoted 3 lines · On Mon, 16 Jul 2007 16:24:12 +0400 Pavel Emelianov <[email protected]> wrote:
When user locks an ipc shmem segmant with
SHM_LOCKctl and the segment is already locked theshmem_lock()function returns 0. After this the subsequent code leaks the existing user struct:
I'm curious. For the past few months, [email protected] have discovered (and fixed) an ongoing stream of obscure but serious and quite long-standing bugs.
How are you discovering these bugs?
Quoted 8 lines
==
ipc/shm.c:sys_shmctl()==... err = shmem_lock(shp->shm_file, 1, user); if (!err) { shp->shm_perm.mode |= SHM_LOCKED; shp->mlock_user = user; } ...
Quoted 5 lines
==
Other results of this are:
1. the newshp->mlock_useris not get-ed and will point to freed memory when the task dies.
That sounds fairly serious - can this lead to memory corruption and crashes?
Quoted 30 lines
2. the
RLIMIT_MEMLOCKis screwed on both user structs.The exploit looks like this:
==
id = shmget(...); setresuid(uid, 0, 0); shmctl(id, SHM_LOCK, NULL); setresuid(uid + 1, 0, 0); shmctl(id, SHM_LOCK, NULL);==
My solution is to return 0 to the userspace and do not change the segment's user.
Signed-off-by Pavel Emelianov <[email protected]>---
ipc/shm.c renamed from ipc/shm.c.shlfix +1
-1@@ -715,7 +715,7 @@ asmlinkage long sys_shmctl (int shmid, istruct user_struct * user = current->user;if (!is_file_hugepages (shp->shm_file)) { err =shmem_lock (shp->shm_file,1 , user); -if (!err) { +if (!err && !(shp->shm_perm.mode &SHM_LOCKED )){ shp->shm_perm.mode |=SHM_LOCKED ; shp->mlock_user = user; }
Quoted 4 lines · Andrew Morton
On Mon, 16 Jul 2007 16:24:12 +0400
Pavel Emelianov <[email protected]> wrote:
Quoted 8 lines
Quoted 3 lines
When user locks an ipc shmem segmant with
SHM_LOCKctl and the segment is already locked theshmem_lock()function returns 0. After this the subsequent code leaks the existing user struct:I'm curious. For the past few months, [email protected] have discovered (and fixed) an ongoing stream of obscure but serious and quite long-standing bugs.
thanks a lot :@)
Quoted 1 line
How are you discovering these bugs?
Not sure what to answer :) Just trying to do our best.
This bug was thought over by Pavel for about 3 month after a single uid leak in container was detected by beancounters' kernel memory accounting...
Quoted 10 lines
Quoted 10 lines
==
ipc/shm.c:sys_shmctl()==... err = shmem_lock(shp->shm_file, 1, user); if (!err) { shp->shm_perm.mode |= SHM_LOCKED; shp->mlock_user = user; } ...==
Quoted 6 lines
Quoted 3 lines
Other results of this are:
1. the newshp->mlock_useris not get-ed and will point to freed memory when the task dies.That sounds fairly serious - can this lead to memory corruption and crashes?
Yes it can. According to Pavel when the shmem segment is destroyed it puts the mlock_user pointer, which can already be stalled.
Kirill
Quoted 14 lines · On Tue, 17 Jul 2007 13:07:55 +0400 Kirill Korotaev <[email protected]> wrote:
Quoted 4 lines · Andrew Morton
On Mon, 16 Jul 2007 16:24:12 +0400
Pavel Emelianov <[email protected]> wrote:Quoted 8 lines
Quoted 3 lines
When user locks an ipc shmem segmant with
SHM_LOCKctl and the segment is already locked theshmem_lock()function returns 0. After this the subsequent code leaks the existing user struct:I'm curious. For the past few months, [email protected] have discovered (and fixed) an ongoing stream of obscure but serious and quite long-standing bugs.
Quoted 5 lines
thanks a lot :@)
Quoted 1 line
How are you discovering these bugs?
Not sure what to answer :) Just trying to do our best.
hm, OK, I was visualising some mysterious Russian bugfinding machine or something.
Don't stop ;)
Quoted 18 lines
This bug was thought over by Pavel for about 3 month after a single uid leak in container was detected by beancounters' kernel memory accounting...
Quoted 15 lines
Quoted 13 lines
==
ipc/shm.c:sys_shmctl()==... err = shmem_lock(shp->shm_file, 1, user); if (!err) { shp->shm_perm.mode |= SHM_LOCKED; shp->mlock_user = user; } ...==
Other results of this are:
1. the newshp->mlock_useris not get-ed and will point to freed memory when the task dies.
Quoted 4 lines
Quoted 1 line
That sounds fairly serious - can this lead to memory corruption and crashes?
Yes it can. According to Pavel when the shmem segment is destroyed it puts the
mlock_userpointer, which can already be stalled.
OK, thanks, I'll feed a copy in [email protected]'s direction.