[PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Manfred Spraul]

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 1: add support for sys_unshare(CLONE_SYSVSEM)

I'm not sure who's the right maintainer, Andrew, could you merge it?

Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
 ipc/sem.c     |    1 +
 kernel/fork.c |   18 ++++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 0b45a4d..35841bd 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
 	undo_list = tsk->sysvsem.undo_list;
 	if (!undo_list)
 		return;
+	tsk->sysvsem.undo_list = NULL;
 
 	if (!atomic_dec_and_test(&undo_list->refcnt))
 		return;
diff --git a/kernel/fork.c b/kernel/fork.c
index 9c042f9..7f242b0 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1675,13 +1675,17 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
 }
 
 /*
- * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
- * supported yet
+ * Unsharing of semundo for tasks created with CLONE_SYSVSEM doesn't require
+ * any allocations: it means that the task leaves the existing undo lists,
+ * just like sys_exit(). The new undo lists are allocated on demand in the
+ * ipc syscalls.
+ * new_ulistp is set to a non-NULL value, the caller expects that on success.
  */
 static int unshare_semundo(unsigned long unshare_flags, struct sem_undo_list **new_ulistp)
 {
-	if (unshare_flags & CLONE_SYSVSEM)
-		return -EINVAL;
+	if (unshare_flags & CLONE_SYSVSEM) {
+		*new_ulistp = (void*)1;
+	}
 
 	return 0;
 }
@@ -1731,6 +1735,12 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
 		goto bad_unshare_cleanup_semundo;
 
 	if (new_fs ||  new_mm || new_fd || new_ulist || new_nsproxy) {
+		if (unshare_flags & CLONE_SYSVSEM) {
+			/*
+			 * CLONE_SYSVSEM is equivalent to sys_exit().
+			 */
+			exit_sem(current);
+		}
 
 		if (new_nsproxy) {
 			switch_task_namespaces(current, new_nsproxy);
-- 
1.5.4.1

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Andrew Morton]

On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul <manfred@colorfullife.com> wrote:

> 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 1: add support for sys_unshare(CLONE_SYSVSEM)
> 

Is this a non-back-compatible change?

> 
> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
> ---
>  ipc/sem.c     |    1 +
>  kernel/fork.c |   18 ++++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 0b45a4d..35841bd 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
>  	undo_list = tsk->sysvsem.undo_list;
>  	if (!undo_list)
>  		return;
> +	tsk->sysvsem.undo_list = NULL;
>  
>  	if (!atomic_dec_and_test(&undo_list->refcnt))
>  		return;
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 9c042f9..7f242b0 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1675,13 +1675,17 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
>  }
>  
>  /*
> - * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
> - * supported yet
> + * Unsharing of semundo for tasks created with CLONE_SYSVSEM doesn't require
> + * any allocations: it means that the task leaves the existing undo lists,
> + * just like sys_exit(). The new undo lists are allocated on demand in the
> + * ipc syscalls.
> + * new_ulistp is set to a non-NULL value, the caller expects that on success.
>   */
>  static int unshare_semundo(unsigned long unshare_flags, struct sem_undo_list **new_ulistp)
>  {
> -	if (unshare_flags & CLONE_SYSVSEM)
> -		return -EINVAL;
> +	if (unshare_flags & CLONE_SYSVSEM) {
> +		*new_ulistp = (void*)1;
> +	}

And can we do anything nicer than this?

>  	return 0;
>  }
> @@ -1731,6 +1735,12 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
>  		goto bad_unshare_cleanup_semundo;
>  
>  	if (new_fs ||  new_mm || new_fd || new_ulist || new_nsproxy) {
> +		if (unshare_flags & CLONE_SYSVSEM) {
> +			/*
> +			 * CLONE_SYSVSEM is equivalent to sys_exit().
> +			 */
> +			exit_sem(current);
> +		}
>  
>  		if (new_nsproxy) {
>  			switch_task_namespaces(current, new_nsproxy);
> -- 
> 1.5.4.1

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 2124 bytes --]

Andrew Morton wrote:
> On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul <manfred@colorfullife.com> wrote:
>
>   
>> 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 1: add support for sys_unshare(CLONE_SYSVSEM)
>>
>>     
>
> Is this a non-back-compatible change?
>
>   
It adds a new feature - previously sys_unshare(CLONE_SYSVSEM) returned 
-EINVAL.

>> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
>> ---
>>  ipc/sem.c     |    1 +
>>  kernel/fork.c |   18 ++++++++++++++----
>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/ipc/sem.c b/ipc/sem.c
>> index 0b45a4d..35841bd 100644
>> --- a/ipc/sem.c
>> +++ b/ipc/sem.c
>> @@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
>>  	undo_list = tsk->sysvsem.undo_list;
>>  	if (!undo_list)
>>  		return;
>> +	tsk->sysvsem.undo_list = NULL;
>>  
>>  	if (!atomic_dec_and_test(&undo_list->refcnt))
>>  		return;
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 9c042f9..7f242b0 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -1675,13 +1675,17 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
>>  }
>>  
>>  /*
>> - * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
>> - * supported yet
>> + * Unsharing of semundo for tasks created with CLONE_SYSVSEM doesn't require
>> + * any allocations: it means that the task leaves the existing undo lists,
>> + * just like sys_exit(). The new undo lists are allocated on demand in the
>> + * ipc syscalls.
>> + * new_ulistp is set to a non-NULL value, the caller expects that on success.
>>   */
>>  static int unshare_semundo(unsigned long unshare_flags, struct sem_undo_list **new_ulistp)
>>  {
>> -	if (unshare_flags & CLONE_SYSVSEM)
>> -		return -EINVAL;
>> +	if (unshare_flags & CLONE_SYSVSEM) {
>> +		*new_ulistp = (void*)1;
>> +	}
>>     
>
> And can we do anything nicer than this?
>
>   
Attached is an alternative. If you prefer it, I'll send another patch set.

--
    Manfred

[-- Attachment #2: patch-step1-alternative --]
[-- Type: text/plain, Size: 2366 bytes --]

diff --git a/ipc/sem.c b/ipc/sem.c
index 0b45a4d..35841bd 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
 	undo_list = tsk->sysvsem.undo_list;
 	if (!undo_list)
 		return;
+	tsk->sysvsem.undo_list = NULL;
 
 	if (!atomic_dec_and_test(&undo_list->refcnt))
 		return;
diff --git a/kernel/fork.c b/kernel/fork.c
index 9c042f9..535aa92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1675,18 +1675,6 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
 }
 
 /*
- * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
- * supported yet
- */
-static int unshare_semundo(unsigned long unshare_flags, struct sem_undo_list **new_ulistp)
-{
-	if (unshare_flags & CLONE_SYSVSEM)
-		return -EINVAL;
-
-	return 0;
-}
-
-/*
  * unshare allows a process to 'unshare' part of the process
  * context which was originally shared using clone.  copy_*
  * functions used by do_fork() cannot be used here directly
@@ -1701,7 +1689,6 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
 	struct sighand_struct *new_sigh = NULL;
 	struct mm_struct *mm, *new_mm = NULL, *active_mm = NULL;
 	struct files_struct *fd, *new_fd = NULL;
-	struct sem_undo_list *new_ulist = NULL;
 	struct nsproxy *new_nsproxy = NULL;
 
 	check_unshare_flags(&unshare_flags);
@@ -1724,13 +1711,17 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
 		goto bad_unshare_cleanup_sigh;
 	if ((err = unshare_fd(unshare_flags, &new_fd)))
 		goto bad_unshare_cleanup_vm;
-	if ((err = unshare_semundo(unshare_flags, &new_ulist)))
-		goto bad_unshare_cleanup_fd;
 	if ((err = unshare_nsproxy_namespaces(unshare_flags, &new_nsproxy,
 			new_fs)))
-		goto bad_unshare_cleanup_semundo;
+		goto bad_unshare_cleanup_fd;
 
-	if (new_fs ||  new_mm || new_fd || new_ulist || new_nsproxy) {
+	if (new_fs ||  new_mm || new_fd || (unshare_flags & CLONE_SYSVSEM) || new_nsproxy) {
+		if (unshare_flags & CLONE_SYSVSEM) {
+			/*
+			 * CLONE_SYSVSEM is equivalent to sys_exit().
+			 */
+			exit_sem(current);
+		}
 
 		if (new_nsproxy) {
 			switch_task_namespaces(current, new_nsproxy);
@@ -1766,7 +1757,6 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
 	if (new_nsproxy)
 		put_nsproxy(new_nsproxy);
 
-bad_unshare_cleanup_semundo:
 bad_unshare_cleanup_fd:
 	if (new_fd)
 		put_files_struct(new_fd);

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Andrew Morton]

On Sun, 13 Apr 2008 13:36:24 +0200 Manfred Spraul <manfred@colorfullife.com> wrote:

> >> +	if (unshare_flags & CLONE_SYSVSEM) {
> >> +		*new_ulistp = (void*)1;
> >> +	}
> >>     
> >
> > And can we do anything nicer than this?
> >
> >   
> Attached is an alternative. If you prefer it, I'll send another patch set.

Well I suppose we can fiddle with this sort of thing later.  Right now the
major concern is the testedness and reviewedness of these changes?

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Serge E. Hallyn]

Quoting Manfred Spraul (manfred@colorfullife.com):
> Andrew Morton wrote:
>> On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul 
>> <manfred@colorfullife.com> wrote:
>>
>>   
>>> 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 1: add support for sys_unshare(CLONE_SYSVSEM)
>>>
>>>     
>>
>> Is this a non-back-compatible change?
>>
>>   
> It adds a new feature - previously sys_unshare(CLONE_SYSVSEM) returned 
> -EINVAL.
>
>>> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
>>> ---
>>>  ipc/sem.c     |    1 +
>>>  kernel/fork.c |   18 ++++++++++++++----
>>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ipc/sem.c b/ipc/sem.c
>>> index 0b45a4d..35841bd 100644
>>> --- a/ipc/sem.c
>>> +++ b/ipc/sem.c
>>> @@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
>>>  	undo_list = tsk->sysvsem.undo_list;
>>>  	if (!undo_list)
>>>  		return;
>>> +	tsk->sysvsem.undo_list = NULL;
>>>   	if (!atomic_dec_and_test(&undo_list->refcnt))
>>>  		return;
>>> diff --git a/kernel/fork.c b/kernel/fork.c
>>> index 9c042f9..7f242b0 100644
>>> --- a/kernel/fork.c
>>> +++ b/kernel/fork.c
>>> @@ -1675,13 +1675,17 @@ static int unshare_fd(unsigned long 
>>> unshare_flags, struct files_struct **new_fdp
>>>  }
>>>   /*
>>> - * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
>>> - * supported yet
>>> + * Unsharing of semundo for tasks created with CLONE_SYSVSEM doesn't 
>>> require
>>> + * any allocations: it means that the task leaves the existing undo 
>>> lists,
>>> + * just like sys_exit(). The new undo lists are allocated on demand in 
>>> the
>>> + * ipc syscalls.
>>> + * new_ulistp is set to a non-NULL value, the caller expects that on 
>>> success.
>>>   */
>>>  static int unshare_semundo(unsigned long unshare_flags, struct 
>>> sem_undo_list **new_ulistp)
>>>  {
>>> -	if (unshare_flags & CLONE_SYSVSEM)
>>> -		return -EINVAL;
>>> +	if (unshare_flags & CLONE_SYSVSEM) {
>>> +		*new_ulistp = (void*)1;
>>> +	}
>>>     
>>
>> And can we do anything nicer than this?
>>
>>   
> Attached is an alternative. If you prefer it, I'll send another patch set.

FWIW I definately far far prefer this version  :)

As for 'maintainers', in the end wrt this code I'd defer to any two of
Pavel, Nadia, and Pierre, each of who I've seen do a great deal of
digging around this code.

(I think I saw some set of these go into -mm so I guess I'll just grab
mmotm and test with that a bit later today)

thanks,
-serge

> --
>    Manfred

> diff --git a/ipc/sem.c b/ipc/sem.c
> index 0b45a4d..35841bd 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
>  	undo_list = tsk->sysvsem.undo_list;
>  	if (!undo_list)
>  		return;
> +	tsk->sysvsem.undo_list = NULL;
> 
>  	if (!atomic_dec_and_test(&undo_list->refcnt))
>  		return;
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 9c042f9..535aa92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1675,18 +1675,6 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp
>  }
> 
>  /*
> - * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
> - * supported yet
> - */
> -static int unshare_semundo(unsigned long unshare_flags, struct sem_undo_list **new_ulistp)
> -{
> -	if (unshare_flags & CLONE_SYSVSEM)
> -		return -EINVAL;
> -
> -	return 0;
> -}
> -
> -/*
>   * unshare allows a process to 'unshare' part of the process
>   * context which was originally shared using clone.  copy_*
>   * functions used by do_fork() cannot be used here directly
> @@ -1701,7 +1689,6 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
>  	struct sighand_struct *new_sigh = NULL;
>  	struct mm_struct *mm, *new_mm = NULL, *active_mm = NULL;
>  	struct files_struct *fd, *new_fd = NULL;
> -	struct sem_undo_list *new_ulist = NULL;
>  	struct nsproxy *new_nsproxy = NULL;
> 
>  	check_unshare_flags(&unshare_flags);
> @@ -1724,13 +1711,17 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
>  		goto bad_unshare_cleanup_sigh;
>  	if ((err = unshare_fd(unshare_flags, &new_fd)))
>  		goto bad_unshare_cleanup_vm;
> -	if ((err = unshare_semundo(unshare_flags, &new_ulist)))
> -		goto bad_unshare_cleanup_fd;
>  	if ((err = unshare_nsproxy_namespaces(unshare_flags, &new_nsproxy,
>  			new_fs)))
> -		goto bad_unshare_cleanup_semundo;
> +		goto bad_unshare_cleanup_fd;
> 
> -	if (new_fs ||  new_mm || new_fd || new_ulist || new_nsproxy) {
> +	if (new_fs ||  new_mm || new_fd || (unshare_flags & CLONE_SYSVSEM) || new_nsproxy) {
> +		if (unshare_flags & CLONE_SYSVSEM) {
> +			/*
> +			 * CLONE_SYSVSEM is equivalent to sys_exit().
> +			 */
> +			exit_sem(current);
> +		}
> 
>  		if (new_nsproxy) {
>  			switch_task_namespaces(current, new_nsproxy);
> @@ -1766,7 +1757,6 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
>  	if (new_nsproxy)
>  		put_nsproxy(new_nsproxy);
> 
> -bad_unshare_cleanup_semundo:
>  bad_unshare_cleanup_fd:
>  	if (new_fd)
>  		put_files_struct(new_fd);

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Andrew Morton]

On Mon, 14 Apr 2008 09:58:40 -0500
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> Quoting Manfred Spraul (manfred@colorfullife.com):
> > Andrew Morton wrote:
> >> On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul 
> >> <manfred@colorfullife.com> wrote:
> >>
> >>> sem_undo_list **new_ulistp)
> >>>  {
> >>> -	if (unshare_flags & CLONE_SYSVSEM)
> >>> -		return -EINVAL;
> >>> +	if (unshare_flags & CLONE_SYSVSEM) {
> >>> +		*new_ulistp = (void*)1;
> >>> +	}
> >>>     
> >>
> >> And can we do anything nicer than this?
> >>
> >>   
> > Attached is an alternative. If you prefer it, I'll send another patch set.
> 
> FWIW I definately far far prefer this version  :)

Oh, OK.

I guess I'll drop what I have.  Manfred, can we please have a new patchset?

We might end up slipping this back to 2.6.25.1.  Would that be a bad thing?

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@linux-foundation.org):
> On Mon, 14 Apr 2008 09:58:40 -0500
> "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> 
> > Quoting Manfred Spraul (manfred@colorfullife.com):
> > > Andrew Morton wrote:
> > >> On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul 
> > >> <manfred@colorfullife.com> wrote:
> > >>
> > >>> sem_undo_list **new_ulistp)
> > >>>  {
> > >>> -	if (unshare_flags & CLONE_SYSVSEM)
> > >>> -		return -EINVAL;
> > >>> +	if (unshare_flags & CLONE_SYSVSEM) {
> > >>> +		*new_ulistp = (void*)1;
> > >>> +	}
> > >>>     
> > >>
> > >> And can we do anything nicer than this?
> > >>
> > >>   
> > > Attached is an alternative. If you prefer it, I'll send another patch set.
> > 
> > FWIW I definately far far prefer this version  :)
> 
> Oh, OK.
> 
> I guess I'll drop what I have.  Manfred, can we please have a new patchset?
> 
> We might end up slipping this back to 2.6.25.1.  Would that be a bad thing?

It'd be unfortunate, but as the existing patch doesn't completely fix
the problem it's probably best to wait anyway.

Manfred, I don't want to step on your toes, but please do let me know if
you don't have time to do the next version.  If you do have time, then
thank you again for spotting+fixing the problem.

thanks,
-serge

Re: [PATCH 1/2] fix sys_unshare()+SEM_UNDO: add support for CLONE_SYSVSEM

[Serge E. Hallyn]

Quoting Serge E. Hallyn (serue@us.ibm.com):
> Quoting Manfred Spraul (manfred@colorfullife.com):
> > Andrew Morton wrote:
> >> On Sun, 13 Apr 2008 10:04:17 +0200 Manfred Spraul 
> >> <manfred@colorfullife.com> wrote:
> >>
> >>   
> >>> 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 1: add support for sys_unshare(CLONE_SYSVSEM)
> >>>
> >>>     
> >>
> >> Is this a non-back-compatible change?
> >>
> >>   
> > It adds a new feature - previously sys_unshare(CLONE_SYSVSEM) returned 
> > -EINVAL.
> >
> >>> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
> >>> ---
> >>>  ipc/sem.c     |    1 +
> >>>  kernel/fork.c |   18 ++++++++++++++----
> >>>  2 files changed, 15 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/ipc/sem.c b/ipc/sem.c
> >>> index 0b45a4d..35841bd 100644
> >>> --- a/ipc/sem.c
> >>> +++ b/ipc/sem.c
> >>> @@ -1298,6 +1298,7 @@ void exit_sem(struct task_struct *tsk)
> >>>  	undo_list = tsk->sysvsem.undo_list;
> >>>  	if (!undo_list)
> >>>  		return;
> >>> +	tsk->sysvsem.undo_list = NULL;
> >>>   	if (!atomic_dec_and_test(&undo_list->refcnt))
> >>>  		return;
> >>> diff --git a/kernel/fork.c b/kernel/fork.c
> >>> index 9c042f9..7f242b0 100644
> >>> --- a/kernel/fork.c
> >>> +++ b/kernel/fork.c
> >>> @@ -1675,13 +1675,17 @@ static int unshare_fd(unsigned long 
> >>> unshare_flags, struct files_struct **new_fdp
> >>>  }
> >>>   /*
> >>> - * Unsharing of semundo for tasks created with CLONE_SYSVSEM is not
> >>> - * supported yet
> >>> + * Unsharing of semundo for tasks created with CLONE_SYSVSEM doesn't 
> >>> require
> >>> + * any allocations: it means that the task leaves the existing undo 
> >>> lists,
> >>> + * just like sys_exit(). The new undo lists are allocated on demand in 
> >>> the
> >>> + * ipc syscalls.
> >>> + * new_ulistp is set to a non-NULL value, the caller expects that on 
> >>> success.
> >>>   */
> >>>  static int unshare_semundo(unsigned long unshare_flags, struct 
> >>> sem_undo_list **new_ulistp)
> >>>  {
> >>> -	if (unshare_flags & CLONE_SYSVSEM)
> >>> -		return -EINVAL;
> >>> +	if (unshare_flags & CLONE_SYSVSEM) {
> >>> +		*new_ulistp = (void*)1;
> >>> +	}
> >>>     
> >>
> >> And can we do anything nicer than this?
> >>
> >>   
> > Attached is an alternative. If you prefer it, I'll send another patch set.
> 
> FWIW I definately far far prefer this version  :)
> 
> As for 'maintainers', in the end wrt this code I'd defer to any two of
> Pavel, Nadia, and Pierre, each of who I've seen do a great deal of
> digging around this code.
> 
> (I think I saw some set of these go into -mm so I guess I'll just grab
> mmotm and test with that a bit later today)
> 
> thanks,
> -serge

Of course if we do go this route, then the unshare(2) manpage will need
an update (so Michael Kerrisk cc:d).

thanks,
-serge