| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20080414211030.GA28378@sergelap.austin.ibm.com>
Date: Mon, 14 Apr 2008 16:10:30 -0500
From: "Serge E. Hallyn" <serue@...ibm.com>
To: Manfred Spraul <manfred@...orfullife.com>
Cc: "Serge E. Hallyn" <serue@...ibm.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Pavel Emelyanov <xemul@...nvz.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Sukadev Bhattiprolu <sukadev@...ibm.com>
Subject: Re: [RFC, PATCH] fix SEM_UNDO with namespaces
Quoting Manfred Spraul (manfred@...orfullife.com):
> 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),
Thanks, Manfred. Basically what I was trying to do, except this appears
to actually work :)
So now when I replace fork+unshare(CLONE_NEWNS|CLONE_NEWIPC) with
clone(CLONE_NEWNS|CLONE_NEWIPC|CLONE_SYSVSEM) (moving the first semop
to before the clone), I do in fact manage to get an oops with the mmotm
I fetched this morning. So I maintain that you do also need to ensure at
copy_namespaces() that if CLONE_NEWIPC is specified, then CLONE_SYSVSEM
is *not* specified (since, this being clone, specifying it means that
the semundo is shared with the parent).
thanks,
-serge
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists