linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] prctl: remove one-shot limitation for changing exe link
@ 2016-09-27 15:39 Stanislav Kinsburskiy
  2016-10-20 15:00 ` Stanislav Kinsburskiy
  0 siblings, 1 reply; 2+ messages in thread
From: Stanislav Kinsburskiy @ 2016-09-27 15:39 UTC (permalink / raw)
  To: peterz, mingo
  Cc: mhocko, keescook, linux-kernel, luto, bsegall, john.stultz,
	matthltc, gorcunov, akpm, xemul, vbabka, ebiederm

This limitation came with the reason to remove "another
way for malicious code to obscure a compromised program and
masquerade as a benign process" by allowing "security-concious program can use
this prctl once during its early initialization to ensure the prctl cannot
later be abused for this purpose":

http://marc.info/?l=linux-kernel&m=133160684517468&w=2

This explanation doesn't look sufficient.
The only thing "exe" link is indicating is the file, used to execve, which is
basically nothing and not reliable immediately after process has returned from
execve system call.

Moreover, to use this feture, all the mappings to previous exe file have to be
unmapped and all the new exe file permissions must be satisfied.

Which means, that changing exe link is very similar to calling execve on the
binary.

The need to remove this limitations comes from migration of NFS mount point,
which is not accessible during restore and replaced by other file system.
Because of this exe link has to be changed twice.

v2:
Rebased on current linux-next

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 include/linux/sched.h |    4 +++-
 kernel/sys.c          |   10 ----------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a1c9b42..ad48b7d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -520,7 +520,9 @@ static inline int get_dumpable(struct mm_struct *mm)
 					/* 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() */
+/* This ine-shot flag is droped due to necessivity of changing exe once again
+ * on NFS restore */
+//#define MMF_EXE_FILE_CHANGED	18	/* see prctl_set_mm_exe_file() */
 
 #define MMF_HAS_UPROBES		19	/* has uprobes */
 #define MMF_RECALC_UPROBES	20	/* MMF_HAS_UPROBES can be wrong */
diff --git a/kernel/sys.c b/kernel/sys.c
index 89d5be4..fd6f508 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1696,16 +1696,6 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
 		fput(exe_file);
 	}
 
-	/*
-	 * 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.
-	 */
-	err = -EPERM;
-	if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
-		goto exit;
-
 	err = 0;
 	/* set the new file, lockless */
 	get_file(exe.file);

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] prctl: remove one-shot limitation for changing exe link
  2016-09-27 15:39 [PATCH v2] prctl: remove one-shot limitation for changing exe link Stanislav Kinsburskiy
@ 2016-10-20 15:00 ` Stanislav Kinsburskiy
  0 siblings, 0 replies; 2+ messages in thread
From: Stanislav Kinsburskiy @ 2016-10-20 15:00 UTC (permalink / raw)
  To: peterz, mingo
  Cc: mhocko, keescook, linux-kernel, luto, bsegall, john.stultz,
	matthltc, gorcunov, akpm, xemul, vbabka, ebiederm

Gentlemen, ping.

Let's decide something, how to get rid of this strange solution.

It doesn't provide the security it was aimed to, looks ugly and 
obfuscates the user of the feature.

It looks like it can be just thrown away.

But if not, please, advice, what should be changed to make is safe and 
solid.


27.09.2016 17:39, Stanislav Kinsburskiy пишет:
> This limitation came with the reason to remove "another
> way for malicious code to obscure a compromised program and
> masquerade as a benign process" by allowing "security-concious program can use
> this prctl once during its early initialization to ensure the prctl cannot
> later be abused for this purpose":
>
> http://marc.info/?l=linux-kernel&m=133160684517468&w=2
>
> This explanation doesn't look sufficient.
> The only thing "exe" link is indicating is the file, used to execve, which is
> basically nothing and not reliable immediately after process has returned from
> execve system call.
>
> Moreover, to use this feture, all the mappings to previous exe file have to be
> unmapped and all the new exe file permissions must be satisfied.
>
> Which means, that changing exe link is very similar to calling execve on the
> binary.
>
> The need to remove this limitations comes from migration of NFS mount point,
> which is not accessible during restore and replaced by other file system.
> Because of this exe link has to be changed twice.
>
> v2:
> Rebased on current linux-next
> de
> ---
>   include/linux/sched.h |    4 +++-
>   kernel/sys.c          |   10 ----------
>   2 files changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index a1c9b42..ad48b7d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -520,7 +520,9 @@ static inline int get_dumpable(struct mm_struct *mm)
>   					/* 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() */
> +/* This ine-shot flag is droped due to necessivity of changing exe once again
> + * on NFS restore */
> +//#define MMF_EXE_FILE_CHANGED	18	/* see prctl_set_mm_exe_file() */
>   
>   #define MMF_HAS_UPROBES		19	/* has uprobes */
>   #define MMF_RECALC_UPROBES	20	/* MMF_HAS_UPROBES can be wrong */
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 89d5be4..fd6f508 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1696,16 +1696,6 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
>   		fput(exe_file);
>   	}
>   
> -	/*
> -	 * 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.
> -	 */
> -	err = -EPERM;
> -	if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
> -		goto exit;
> -
>   	err = 0;
>   	/* set the new file, lockless */
>   	get_file(exe.file);
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-10-21  3:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27 15:39 [PATCH v2] prctl: remove one-shot limitation for changing exe link Stanislav Kinsburskiy
2016-10-20 15:00 ` Stanislav Kinsburskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).