linux-kernel - Re: [RFC, PATCH] fix SEM

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 3 Apr 2008 14:44:18 -0500
From:	"Serge E. Hallyn" <serue@...ibm.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Pavel Emelyanov <xemul@...nvz.org>,
	Manfred Spraul <manfred@...orfullife.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Serge Hallyn <serue@...ibm.com>,
	Sukadev Bhattiprolu <sukadev@...ibm.com>
Subject: Re: [RFC, PATCH] fix SEM_UNDO with namespaces

Quoting Eric W. Biederman (ebiederm@...ssion.com):
> Pavel Emelyanov <xemul@...nvz.org> writes:
> >>> I agree, that we should probably destroy this one when the task calls 
> >>> unshare, but trying to keep this list relevant is useless.
> >>>   
> >> A very tricky question: Let's assume we have a process with two threads.
> >> The undo structure is shared, as per opengroup standard.
> >> Now one thread calls unshare(CLONE_NEWIPC). What should happen? We 
> >> cannot destroy the undo structure, the other thread might be still 
> >> interested in it.
> >> If we allow sys_unshare() for multithreaded processes with CLONE_NEWIPC 
> >> and without CLONE_SYSVSEM, then we must handle this case.
> >
> > Hm... I'd simply disable creating any new namespaces for threads.
> > I think other namespaces developers agree with me. Serge, Suka, Eric
> > what do you think?
> 
> I almost agree.  sys_unshare() in a multithreaded process breaks
> all kinds of user space libs.  So you can only reasonably look at
> the problem as what we do with linux tasks that share some things
> and not others.  The posix/opengroup notion of processes and threads
> are a distraction.
> 
> In this case requiring it appears that to require unsharing both
> CLONE_SYSVSEM and CLONE_NEWIPC at the same time.  (i.e. unshare
> of CLONE_SYSVSEM should fail if CLONE_NEWIPC is not also specified).
> 
> Then to make it work we make unshare of SYSVSEM succeed when it is
> not shared.
> 
> This looks like about a 5 line patch or two.
> 
> The effect is because we don't support unsharing of SYSVSEM currently
> we don't support a threaded process unsharing the ipc namespace.
> 
> Eric

Eric, does the following patch correctly interpret your recommendation?

Pavel does it make sense to you?

thanks,
-serge

>From 9c85fb3cb80cea1d888c3c253a9fb6e9bc173649 Mon Sep 17 00:00:00 2001
From: Serge E. Hallyn <serue@...ibm.com>
Date: Thu, 3 Apr 2008 12:43:23 -0700
Subject: [PATCH 1/1] ipc namespaces: fix svsem unsharing issue

Refuse to unshare an ipcns if the semundo is shared and we
are not requesting a new SYSVSEM

Signed-off-by: Serge E. Hallyn <serue@...ibm.com>
---
 ipc/namespace.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

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)) {
+		if (!current->sysvsem.undo_list)
+			goto ok;
+		if (atomic_read(&current->sysvsem.undo_list->refcnt) == 1)
+			goto ok;
+		put_ipc_ns(ns);
+		return ERR_PTR(-EINVAL);
+	}
+
+ok:
 	new_ns = clone_ipc_ns(ns);
 
 	put_ipc_ns(ns);
-- 
1.5.3.6

--
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/