Messages in this thread | | | Date | Mon, 14 Apr 2008 20:40:18 +0200 | From | Manfred Spraul <> | Subject | Re: [PATCH 2/2] fix sys_unshare()+SEM_UNDO: perform an implicit CLONE_SYSVSEM in CLONE_NEWIPC |
| |
Serge E. Hallyn wrote: > Quoting Manfred Spraul (manfred@colorfullife.com): > >> sys_unshare(CLONE_NEWIPC) doesn't handle the undo lists properly, this can >> cause a kernel memory corruption. CLONE_NEWIPC must detach from the existing >> undo lists. >> Fix, part 2: perform an implicit CLONE_SYSVSEM in CLONE_NEWIPC. >> CLONE_NEWIPC creates a new IPC namespace, the task cannot access the >> existing semaphore arrays after the unshare syscall. Thus the task >> can/must detach from the existing undo list entries, too. >> >> This fixes the kernel corruption, because it makes it impossible that >> undo records from two different namespaces are in sysvsem.undo_list. >> > > So this was never an issue with clone(CLONE_NEWIPC|CLONE_SYSVSEM), which > should have had the same result as unshare(CLONE_NEWIPC&~CLONE_SYSVSEM)? > > Actually, the story is slightly different: unshare(CLONE_NEWIPC|CLONE_SYSVSEM) returns -EINVAL right now.
Thus all apps right now call unshare(CLONE_NEWIPC|&~CLONE_SYSVSEM). This combination doesn't make much sense. Even worse - it easily causes a kernel oops. Thus my fix is twofold: - add support for unshare(CLONE_SYSVSEM). - implicitely add CLONE_SYSVSEM to all calls that set CLONE_NEWIPC.
It's not really pretty: If a pivot_namespace syscall is ever added, then CLONE_NEWIPC&~CLONE_SYSVSEM would make sense again. What do you think? Can we break backward compatibility and add if ( (unshare_flags & CLONE_NEWIPC) && !(unshare_flags & CLONE_SYSVSEM) ) return -EINVAL; into sys_unshare()? I have decided against that, it breaks the current ABI. And we gain virtually nothing - most if not all unshare users will be single threaded apps that do not use sysvsem at all, and even most sysvsem users do not use SEM_UNDO.
-- Manfred
| |