lkml.org 
[lkml]   [2008]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC, PATCH] fix SEM_UNDO with namespaces
Serge E. Hallyn wrote:
>> diff --git a/ipc/namespace.c b/ipc/namespace.c
>> index 9171d94..9044505 100644
>> --- a/ipc/namespace.c
>> +++ b/ipc/namespace.c
>> @@ -48,6 +48,16 @@ struct ipc_namespace *copy_ipcs(unsigned long flags, struct ipc_namespace *ns)
>> if (!(flags & CLONE_NEWIPC))
>> return ns;
>>
>> + if (!(flags & CLONE_SYSVSEM)) {
>>
>
> Wait, this should be opposite, right?
>
> [snip]
> Manfred, I'm trying to test this, but can't get an error without this
> patch. Do you have a testcase?

The patch is bogus, sorry that I didn't notice it immediately.
The problem is ipc/sem.c, function find_undo, lookup_undo:
lookup_undo doesn't check the namespace pointer, thus a simple single
threaded app can trigger the problem.

Attached is a test app and a kernel patch that shows the problem.
Run the test app immediately after boot (or within a new ipc namespace),
otherwise the ipc sequence counter will prevent the app from triggering
the problem: The undo structure that was created before unshare() [i.e.
with 1 semaphore in it] will be used after unshare() [i.e. semaphore 100
will be accessed].

With kernel debugging (full slub debugging) enabled, I even got an oops
when I tried to ipcrm the left over array after running undons, probably
because the undo structure was freed at exit_sem() within the new
namespace, but still used in the outer namespace.

--
Manfred
/*
* Copyright (C) 1999,2001 by Manfred Spraul.
*
* Redistribution of this file is permitted under the terms of the GNU
* General Public License (GPL)
* $Header: /home/manfred/cvs-tree/manfred/ipcsem/undotest.c,v 1.2 2003/06/28 15:19:43 manfred Exp $
*/

#ifdef __LINUX__
#define _GNU_SOURCE
#include <sched.h>
#include <wait.h>

#define CLONE_NEWIPC 0x08000000 /* New ipcs */
#endif
#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/sem.h>
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <assert.h>
#include <signal.h>
#include <time.h>
#include <unistd.h>
#include <pthread.h>

union semun {
int val;
struct semid_ds *buf;
unsigned short int *array;
struct seminfo* __buf;
};

int getval(char * str, int id)
{
union semun arg;
int res;

res = semctl(id,0,GETVAL,arg);
if(res==-1) {
printf("GETVAL failed for %s.\n", str);
exit(4);
}
printf(" %s: GETVAL now %d.\n",str, res);
return res;
}

void setzero(int id)
{
union semun arg;
int res;

arg.val = 0;
res = semctl(id,0,SETVAL,arg);
if(res==-1) {
printf("SETVAL failed, errno %d.\n", errno);
exit(4);
}
printf(" SETVAL succeeded.\n");
}


/* test1: verify that undo works at all. */
int main(int argc,char** argv)
{
int res, id;

printf(" ****************************************\n");

/* create array */
res = semget(IPC_PRIVATE, 1, 0700 | IPC_CREAT);
printf(" got semaphore array %xh.\n",res);
if(res == -1) {
printf(" create failed.\n");
return 1;
}
id = res;

setzero(id);

res = getval("test1 init", id);
if (res != 0) {
printf("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\n");
printf("Bad: got unexpected value.\n");
exit(99);
}

/* create sub-process */
res = fork();
if (res < 0) {
printf("Fork failed (errno=%d). Aborting.\n", errno);
res = semctl(id,1,IPC_RMID,NULL);
exit(1);
}
fflush(stdout);
if (!res) {
struct sembuf sop[1];
int id2;

sop[0].sem_num=0;
sop[0].sem_op=1;
sop[0].sem_flg=SEM_UNDO;

res = semop(id,sop,1);
if(res==-1) {
printf("semop failed.\n");
exit(1);
}
res = getval("before unshare()", id);
if (res != 1) {
printf("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\n");
printf("Bad: got unexpected value.\n");
exit(99);
}
fflush(stdout);

errno = 0;
res = unshare(CLONE_NEWNS|CLONE_NEWIPC);
printf(" unshare returned %d, errno now %d (expected: 0,0).\n", res, errno);
fflush(stdout);

/* create array */
res = semget(IPC_PRIVATE, 101, 0700 | IPC_CREAT);
printf(" got semaphore array %xh.\n",res);
if(res == -1) {
printf(" create failed.\n");
return 1;
}
id2 = res;
printf("Now operating on id1=%x, id2=%x. (if both are equal, there will be a memory corruption)\n", id, id2);

/* child: */
sop[0].sem_num=100;
sop[0].sem_op=1;
sop[0].sem_flg=SEM_UNDO;

// This operation should trash kernel memory (if id1==id2)
res = semop(id2,sop,1);
if(res==-1) {
printf("semop failed.\n");
exit(1);
}
exit(1);
} else {
int retval;
retval = wait4(res, NULL, 0, NULL);
if (retval != res) {
printf("wait4 returned unexpeted value %d, errno now %d.\n",
retval, errno);
}
res = getval("after exit", id);
if (res != 0) {
printf("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\n");
printf("Bad: got unexpected value.\n");
return 1;
}
}

return 0;
}
diff --git a/ipc/sem.c b/ipc/sem.c
index 0b45a4d..5c882f9 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1081,6 +1082,7 @@ static struct sem_undo *find_undo(struct ipc_namespace *ns, int semid)
new->id_next = sma->undo;
sma->undo = new;
sem_unlock(sma);
+printk(KERN_ERR "find_undo(%p, %x) new undop %p (for %d sems).\n", ns, semid, new, nsems);
un = new;
spin_unlock(&ulp->lock);
out:
@@ -1153,6 +1155,7 @@ retry_undos:
error = PTR_ERR(sma);
goto out_free;
}
+printk(KERN_ERR "find_undo(%p, %x) == %p, using %ld sems.\n", ns, semid, un, sma->sem_nsems);

/*
* semid identifiers are not unique - find_undo may have
\
 
 \ /
  Last update: 2008-04-06 17:15    [W:0.098 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site