+ c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch added to -mm tree

+ c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch added to -mm tree

[akpm]

The patch titled
     Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal
has been added to the -mm tree.  Its filename is
     c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: Konstantin Khlebnikov <khlebnikov@openvz.org>
Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal

[ fix for "c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-v2" from mm tree ]

After removing mm->num_exe_file_vmas kernel keeps mm->exe_file until final
mmput(), it never becomes NULL while task is alive.

We can check for other mapped files in mm instead of checking
mm->num_exe_file_vmas, and mark mm with flag MMF_EXE_FILE_CHANGED in order
to forbid second changing of mm->exe_file.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Matt Helsley <matthltc@us.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/sched.h |    1 +
 kernel/sys.c          |   31 +++++++++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff -puN include/linux/sched.h~c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal include/linux/sched.h
--- a/include/linux/sched.h~c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal
+++ a/include/linux/sched.h
@@ -437,6 +437,7 @@ extern int get_dumpable(struct mm_struct
 					/* leave room for more dump flags */
 #define MMF_VM_MERGEABLE	16	/* KSM may merge identical pages */
 #define MMF_VM_HUGEPAGE		17	/* set when VM_HUGEPAGE is set on vma */
+#define MMF_EXE_FILE_CHANGED	18	/* see prctl_set_mm_exe_file() */
 
 #define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK)
 
diff -puN kernel/sys.c~c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal kernel/sys.c
--- a/kernel/sys.c~c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal
+++ a/kernel/sys.c
@@ -1714,17 +1714,11 @@ static bool vma_flags_mismatch(struct vm
 
 static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
 {
+	struct vm_area_struct *vma;
 	struct file *exe_file;
 	struct dentry *dentry;
 	int err;
 
-	/*
-	 * Setting new mm::exe_file is only allowed when no VM_EXECUTABLE vma's
-	 * remain. So perform a quick test first.
-	 */
-	if (mm->num_exe_file_vmas)
-		return -EBUSY;
-
 	exe_file = fget(fd);
 	if (!exe_file)
 		return -EBADF;
@@ -1745,17 +1739,30 @@ static int prctl_set_mm_exe_file(struct 
 	if (err)
 		goto exit;
 
+	down_write(&mm->mmap_sem);
+
+	/*
+	 * Forbid mm->exe_file change if there are mapped other files.
+	 */
+	err = -EBUSY;
+	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		if (vma->vm_file && !path_equal(&vma->vm_file->f_path,
+						&exe_file->f_path))
+			goto exit_unlock;
+	}
+
 	/*
 	 * The symlink can be changed only once, just to disallow arbitrary
 	 * transitions malicious software might bring in. This means one
 	 * could make a snapshot over all processes running and monitor
 	 * /proc/pid/exe changes to notice unusual activity if needed.
 	 */
-	down_write(&mm->mmap_sem);
-	if (likely(!mm->exe_file))
-		set_mm_exe_file(mm, exe_file);
-	else
-		err = -EBUSY;
+	err = -EPERM;
+	if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
+		goto exit_unlock;
+
+	set_mm_exe_file(mm, exe_file);
+exit_unlock:
 	up_write(&mm->mmap_sem);
 
 exit:
_
Subject: Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal

Patches currently in -mm which might be from khlebnikov@openvz.org are

mm-hugetlb-fix-warning-in-alloc_huge_page-dequeue_huge_page_vma.patch
linux-next.patch
mm-correctly-synchronize-rss-counters-at-exit-exec.patch
mm-correctly-synchronize-rss-counters-at-exit-exec-fix.patch
mm-correctly-synchronize-rss-counters-at-exit-exec-set-task-exit-code-before-complete_vfork_done.patch
mm-remove-swap-token-code.patch
mm-vmscan-remove-lumpy-reclaim.patch
mm-vmscan-do-not-stall-on-writeback-during-memory-compaction.patch
mm-vmscan-remove-reclaim_mode_t.patch
mm-memcg-scanning_global_lru-means-mem_cgroup_disabled.patch
mm-memcg-move-reclaim_stat-into-lruvec.patch
mm-push-lru-index-into-shrink_active_list.patch
mm-push-lru-index-into-shrink_active_list-fix.patch
mm-mark-mm-inline-functions-as-__always_inline.patch
mm-remove-lru-type-checks-from-__isolate_lru_page.patch
mm-memcg-kill-mem_cgroup_lru_del.patch
mm-memcg-use-vm_swappiness-from-target-memory-cgroup.patch
fork-call-complete_vfork_done-after-clearing-child_tid-and-flushing-rss-counters.patch
c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Oleg Nesterov]

On 04/19, Andrew Morton wrote:
>
> From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal
>
> [ fix for "c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-v2" from mm tree ]
>
> After removing mm->num_exe_file_vmas kernel keeps mm->exe_file until final
> mmput(), it never becomes NULL while task is alive.
>
> We can check for other mapped files in mm instead of checking
> mm->num_exe_file_vmas, and mark mm with flag MMF_EXE_FILE_CHANGED in order
> to forbid second changing of mm->exe_file.

I lost the track a long ago.

Just one question, what does this "forbid second changing" actually mean?

>  	 * The symlink can be changed only once, just to disallow arbitrary
>  	 * transitions malicious software might bring in. This means one
>  	 * could make a snapshot over all processes running and monitor
>  	 * /proc/pid/exe changes to notice unusual activity if needed.
>  	 */
> -	down_write(&mm->mmap_sem);
> -	if (likely(!mm->exe_file))
> -		set_mm_exe_file(mm, exe_file);
> -	else
> -		err = -EBUSY;
> +	err = -EPERM;
> +	if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
> +		goto exit_unlock;
> +
> +	set_mm_exe_file(mm, exe_file);
> +exit_unlock:

OK, I am not arguing, but looking at this code I suspect that you
also want to forbid the second change after fork too?

If yes, then you probably need to include MMF_EXE_FILE_CHANGED in
MMF_INIT_MASK.

But at the same time, then it should be probably cleared somewhere
in bprm_mm_init() paths.

Oleg.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Cyrill Gorcunov]

On Thu, Apr 19, 2012 at 09:20:05PM +0200, Oleg Nesterov wrote:
> On 04/19, Andrew Morton wrote:
> >
> > From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> > Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal
> >
> > [ fix for "c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-v2" from mm tree ]
> >
> > After removing mm->num_exe_file_vmas kernel keeps mm->exe_file until final
> > mmput(), it never becomes NULL while task is alive.
> >
> > We can check for other mapped files in mm instead of checking
> > mm->num_exe_file_vmas, and mark mm with flag MMF_EXE_FILE_CHANGED in order
> > to forbid second changing of mm->exe_file.
> 
> I lost the track a long ago.
> 
> Just one question, what does this "forbid second changing" actually mean?

Heh :) Oleg, it was actually your idea to make this feature "one-shot".
Once exe-file changed to a new value, it can't be changed again. The
reason was to bring at least minimum disturbance in sysadmins life.

	Cyrill

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Oleg Nesterov]

On 04/20, Cyrill Gorcunov wrote:
>
> On Thu, Apr 19, 2012 at 09:20:05PM +0200, Oleg Nesterov wrote:
> > On 04/19, Andrew Morton wrote:
> > >
> > > From: Konstantin Khlebnikov <khlebnikov@openvz.org>
> > > Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal
> > >
> > > [ fix for "c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-v2" from mm tree ]
> > >
> > > After removing mm->num_exe_file_vmas kernel keeps mm->exe_file until final
> > > mmput(), it never becomes NULL while task is alive.
> > >
> > > We can check for other mapped files in mm instead of checking
> > > mm->num_exe_file_vmas, and mark mm with flag MMF_EXE_FILE_CHANGED in order
> > > to forbid second changing of mm->exe_file.
> >
> > I lost the track a long ago.
> >
> > Just one question, what does this "forbid second changing" actually mean?
>
> Heh :) Oleg, it was actually your idea to make this feature "one-shot".

Heh, no ;)

IIRC, I only asked you what do you actually want,

	Just one note for the record, prctl_set_mm_exe_file() does

		if (mm->num_exe_file_vmas)
			return -EBUSY;

	We could do

		if (mm->exe_file)
			return -EBUSY;

	This way "because this feature is a special to C/R" becomes
	really true. IOW, you can't do PR_SET_MM_EXE_FILE twice.

	I am fine either way, just I want to ensure you really want
	the current version.

and only because it was documented as "feature is a special to C/R".

> Once exe-file changed to a new value, it can't be changed again. The
> reason was to bring at least minimum disturbance in sysadmins life.

You misunderstood. I am not arguing with "one-shot", I do not really
care.

My question is: unless I missed something "it can't be changed again"
is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
by design?

Oleg.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Cyrill Gorcunov]

On Thu, Apr 19, 2012 at 11:12:16PM +0200, Oleg Nesterov wrote:
> > Heh :) Oleg, it was actually your idea to make this feature "one-shot".
> 
> Heh, no ;)
> 
> IIRC, I only asked you what do you actually want,
> 
> 	Just one note for the record, prctl_set_mm_exe_file() does
> 
> 		if (mm->num_exe_file_vmas)
> 			return -EBUSY;
> 
> 	We could do
> 
> 		if (mm->exe_file)
> 			return -EBUSY;
> 
> 	This way "because this feature is a special to C/R" becomes
> 	really true. IOW, you can't do PR_SET_MM_EXE_FILE twice.
> 
> 	I am fine either way, just I want to ensure you really want
> 	the current version.
> 
> and only because it was documented as "feature is a special to C/R".

ok, ubedil :)

> > Once exe-file changed to a new value, it can't be changed again. The
> > reason was to bring at least minimum disturbance in sysadmins life.
> 
> You misunderstood. I am not arguing with "one-shot", I do not really
> care.
> 
> My question is: unless I missed something "it can't be changed again"
> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
> by design?

Hmm, not sure, Konstantin?

	Cyrill

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Konstantin Khlebnikov]

Cyrill Gorcunov wrote:
> On Thu, Apr 19, 2012 at 11:12:16PM +0200, Oleg Nesterov wrote:
>>> Heh :) Oleg, it was actually your idea to make this feature "one-shot".
>>
>> Heh, no ;)
>>
>> IIRC, I only asked you what do you actually want,
>>
>> 	Just one note for the record, prctl_set_mm_exe_file() does
>>
>> 		if (mm->num_exe_file_vmas)
>> 			return -EBUSY;
>>
>> 	We could do
>>
>> 		if (mm->exe_file)
>> 			return -EBUSY;
>>
>> 	This way "because this feature is a special to C/R" becomes
>> 	really true. IOW, you can't do PR_SET_MM_EXE_FILE twice.
>>
>> 	I am fine either way, just I want to ensure you really want
>> 	the current version.
>>
>> and only because it was documented as "feature is a special to C/R".
>
> ok, ubedil :)
>
>>> Once exe-file changed to a new value, it can't be changed again. The
>>> reason was to bring at least minimum disturbance in sysadmins life.
>>
>> You misunderstood. I am not arguing with "one-shot", I do not really
>> care.
>>
>> My question is: unless I missed something "it can't be changed again"
>> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
>> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
>> by design?
>
> Hmm, not sure, Konstantin?

Why not? It has new pid, why it cannot change exe_file? Actually I don't care too.
But even if we include this bit into MMF_INIT_MASK we cannot forbid exe-file change
in childs tasks which was forked before exe-file change in parent.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Cyrill Gorcunov]

On Fri, Apr 20, 2012 at 02:08:43AM +0400, Konstantin Khlebnikov wrote:
> >>My question is: unless I missed something "it can't be changed again"
> >>is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
> >>the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
> >>by design?
> >
> >Hmm, not sure, Konstantin?
> 
> Why not? It has new pid, why it cannot change exe_file? Actually I don't care too.
> But even if we include this bit into MMF_INIT_MASK we cannot forbid exe-file change
> in childs tasks which was forked before exe-file change in parent.

OK, I see, thanks.

	Cyrill

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Oleg Nesterov]

On 04/20, Konstantin Khlebnikov wrote:
>
>>> My question is: unless I missed something "it can't be changed again"
>>> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
>>> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
>>> by design?
>>
>> Hmm, not sure, Konstantin?
>
> Why not? It has new pid, why it cannot change exe_file?

OK, if you do not see a problem - I agree with this patch.

I don't really understand what PR_SET_MM_EXE_FILE buys us if
parent/child can have different /proc/pid/exe links without
exec, but

> Actually I don't care too.

same here ;)

and at least this addresses the "establishes a bound on how
frequently the symlink can change" from Matt.

Oleg.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Konstantin Khlebnikov]

Oleg Nesterov wrote:
> On 04/20, Cyrill Gorcunov wrote:
>>
>> On Thu, Apr 19, 2012 at 09:20:05PM +0200, Oleg Nesterov wrote:
>>> On 04/19, Andrew Morton wrote:
>>>>
>>>> From: Konstantin Khlebnikov<khlebnikov@openvz.org>
>>>> Subject: c/r: prctl: update prctl_set_mm_exe_file() after mm->num_exe_file_vmas removal
>>>>
>>>> [ fix for "c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-v2" from mm tree ]
>>>>
>>>> After removing mm->num_exe_file_vmas kernel keeps mm->exe_file until final
>>>> mmput(), it never becomes NULL while task is alive.
>>>>
>>>> We can check for other mapped files in mm instead of checking
>>>> mm->num_exe_file_vmas, and mark mm with flag MMF_EXE_FILE_CHANGED in order
>>>> to forbid second changing of mm->exe_file.
>>>
>>> I lost the track a long ago.
>>>
>>> Just one question, what does this "forbid second changing" actually mean?
>>
>> Heh :) Oleg, it was actually your idea to make this feature "one-shot".
>
> Heh, no ;)
>
> IIRC, I only asked you what do you actually want,
>
> 	Just one note for the record, prctl_set_mm_exe_file() does
>
> 		if (mm->num_exe_file_vmas)
> 			return -EBUSY;
>
> 	We could do
>
> 		if (mm->exe_file)
> 			return -EBUSY;
>
> 	This way "because this feature is a special to C/R" becomes
> 	really true. IOW, you can't do PR_SET_MM_EXE_FILE twice.
>
> 	I am fine either way, just I want to ensure you really want
> 	the current version.
>
> and only because it was documented as "feature is a special to C/R".
>
>> Once exe-file changed to a new value, it can't be changed again. The
>> reason was to bring at least minimum disturbance in sysadmins life.
>
> You misunderstood. I am not arguing with "one-shot", I do not really
> care.
>
> My question is: unless I missed something "it can't be changed again"
> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
> by design?
>
> Oleg.
>

I found more weird case: child thread (with CLONE_THREAD and without CLONE_VM)
changes its exe_file...

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Oleg Nesterov]

On 04/20, Konstantin Khlebnikov wrote:
>
> Oleg Nesterov wrote:
>>
>> You misunderstood. I am not arguing with "one-shot", I do not really
>> care.
>>
>> My question is: unless I missed something "it can't be changed again"
>> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
>> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
>> by design?
>>
>> Oleg.
>>
>
> I found more weird case: child thread (with CLONE_THREAD and without CLONE_VM)
> changes its exe_file...

No. copy_process() does:

	if ((clone_flags & CLONE_THREAD) && !(clone_flags & CLONE_SIGHAND))
		return ERR_PTR(-EINVAL);

	if ((clone_flags & CLONE_SIGHAND) && !(clone_flags & CLONE_VM))
		return ERR_PTR(-EINVAL);

IOW, CLONE_THREAD => CLONE_SIGHAND => CLONE_VM

Oleg.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Cyrill Gorcunov]

On Thu, Apr 19, 2012 at 11:51:09PM +0200, Oleg Nesterov wrote:
> On 04/20, Konstantin Khlebnikov wrote:
> >
> > Oleg Nesterov wrote:
> >>
> >> You misunderstood. I am not arguing with "one-shot", I do not really
> >> care.
> >>
> >> My question is: unless I missed something "it can't be changed again"
> >> is not actually true. A task does PR_SET_MM_EXE_FILE, then it forks
> >> the new child. The child can do PR_SET_MM_EXE_FILE again. Is this
> >> by design?
> >>
> >> Oleg.
> >>
> >
> > I found more weird case: child thread (with CLONE_THREAD and without CLONE_VM)
> > changes its exe_file...
> 
> No. copy_process() does:
> 
> 	if ((clone_flags & CLONE_THREAD) && !(clone_flags & CLONE_SIGHAND))
> 		return ERR_PTR(-EINVAL);
> 
> 	if ((clone_flags & CLONE_SIGHAND) && !(clone_flags & CLONE_VM))
> 		return ERR_PTR(-EINVAL);
> 
> IOW, CLONE_THREAD => CLONE_SIGHAND => CLONE_VM

Guys, while I more-less agree with Matt about single-shot behaviour

[ let me copy my and his email

  >> With mm->exe_file this prctl option become a one-shot
  >> only, and while at moment our user-space tool can perfectly
  >> live with that I thought that there is no strict need to
  >> limit the option this way from the very beginning.
  >>
  > As far as backward compatibility, isn't it better to lift that restriction
  > later rather than add it? I think the latter would very likely "break"
  > things whereas the former would not.
  >
  > I also prefer that restriction because it establishes a bound on how
  > frequently the symlink can change. Keeping it a one-shot deal makes the
  > values that show up in tools like top more reliable for admins.
]

I guess maybe it's time to drop one-shot requirement and as result
we can drop MMF_EXE_FILE_CHANGED bit completely, making overall code
simplier?

Our tool can live with any behaviour (and one shot and multishot,
doesn't matter).

	Cyrill

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Oleg Nesterov]

On 04/20, Cyrill Gorcunov wrote:
>
> Guys, while I more-less agree with Matt about single-shot behaviour
>
> [ let me copy my and his email
>
>   >> With mm->exe_file this prctl option become a one-shot
>   >> only, and while at moment our user-space tool can perfectly
>   >> live with that I thought that there is no strict need to
>   >> limit the option this way from the very beginning.
>   >>
>   > As far as backward compatibility, isn't it better to lift that restriction
>   > later rather than add it? I think the latter would very likely "break"
>   > things whereas the former would not.
>   >
>   > I also prefer that restriction because it establishes a bound on how
>   > frequently the symlink can change. Keeping it a one-shot deal makes the
>   > values that show up in tools like top more reliable for admins.
> ]
>
> I guess maybe it's time to drop one-shot requirement and as result
> we can drop MMF_EXE_FILE_CHANGED bit completely,

Plus perhaps we can remove this for_each_vma check?

> making overall code
> simplier?

Personally I'd certainly prefer this ;)



But let me repeat to avoid the confusion. I am fine either way,
I am not going to discuss this again unless I see something which
looks technically wrong. And the current MMF_EXE_FILE_CHANGED
doesn't look right even if the problem is minor.

Oleg.

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Konstantin Khlebnikov]

Oleg Nesterov wrote:
> On 04/20, Cyrill Gorcunov wrote:
>>
>> Guys, while I more-less agree with Matt about single-shot behaviour
>>
>> [ let me copy my and his email
>>
>>    >>  With mm->exe_file this prctl option become a one-shot
>>    >>  only, and while at moment our user-space tool can perfectly
>>    >>  live with that I thought that there is no strict need to
>>    >>  limit the option this way from the very beginning.
>>    >>
>>    >  As far as backward compatibility, isn't it better to lift that restriction
>>    >  later rather than add it? I think the latter would very likely "break"
>>    >  things whereas the former would not.
>>    >
>>    >  I also prefer that restriction because it establishes a bound on how
>>    >  frequently the symlink can change. Keeping it a one-shot deal makes the
>>    >  values that show up in tools like top more reliable for admins.
>> ]
>>
>> I guess maybe it's time to drop one-shot requirement and as result
>> we can drop MMF_EXE_FILE_CHANGED bit completely,
>
> Plus perhaps we can remove this for_each_vma check?
>
>> making overall code
>> simplier?
>
> Personally I'd certainly prefer this ;)
>
>
>
> But let me repeat to avoid the confusion. I am fine either way,
> I am not going to discuss this again unless I see something which
> looks technically wrong. And the current MMF_EXE_FILE_CHANGED
> doesn't look right even if the problem is minor.

Yeah, whole this protection does not protect anything and can be easily bypassed.
For example task can re-execute itself and change exe-file again and again.

>
> Oleg.
>

Re: + c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm- num_exe_file_vmas-removal.patch added to -mm tree

[Cyrill Gorcunov]

On Fri, Apr 20, 2012 at 12:09:19AM +0200, Oleg Nesterov wrote:
> >
> > I guess maybe it's time to drop one-shot requirement and as result
> > we can drop MMF_EXE_FILE_CHANGED bit completely,
> 
> Plus perhaps we can remove this for_each_vma check?
>
> > making overall code simplier?
> 
> Personally I'd certainly prefer this ;)
> 
> But let me repeat to avoid the confusion. I am fine either way,
> I am not going to discuss this again unless I see something which
> looks technically wrong. And the current MMF_EXE_FILE_CHANGED
> doesn't look right even if the problem is minor.

So if there no stong agrues against, lets rip all together --
and for_each_vma and MMF_EXE_FILE_CHANGED bits, finally making
code simplier.

	Cyrill