2012-05-31 15:41:58

by Borislav Petkov

[permalink] [raw]
Subject: Re: CROSS_MEMORY_ATTACH default y?

+ LKML.

On Thu, May 31, 2012 at 05:35:12PM +0200, Borislav Petkov wrote:
> Hi,
>
> can you please explain why CROSS_MEMORY_ATTACH is default y? Why should
> those process_vm_{readv,writev} syscalls be enabled by default?
>
> Thanks.

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551


2012-06-01 01:07:09

by Christopher Yeoh

[permalink] [raw]
Subject: Re: CROSS_MEMORY_ATTACH default y?

On Thu, 31 May 2012 17:42:24 +0200
Borislav Petkov <[email protected]> wrote:

> + LKML.
>
> On Thu, May 31, 2012 at 05:35:12PM +0200, Borislav Petkov wrote:
> > Hi,
> >
> > can you please explain why CROSS_MEMORY_ATTACH is default y? Why
> > should those process_vm_{readv,writev} syscalls be enabled by
> > default?

There was a bit of a discussion at the time I submitted the patch:

https://lkml.org/lkml/2012/4/23/606

Basically CMA had been in for a while already in a released kernel
before I submitted a patch to allow it to be disabled. So the patch
preserves existing behaviour.

Regards,

Chris
--
[email protected]

2012-06-05 11:05:18

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH] CMA: Do no enable it by default

From: Borislav Petkov <[email protected]>
Date: Tue, 5 Jun 2012 12:52:01 +0200
Subject: [PATCH] CMA: Do no enable it by default


From: Borislav Petkov <[email protected]>
Date: Tue, 5 Jun 2012 12:52:01 +0200
Subject: [PATCH] CMA: Do no enable it by default

CROSS_MEMORY_ATTACH is a MPI feature which shouldn't be enabled by
default on every linux system simply because the majority of users do
not need it.

Besides, in the config option it says "... which allow a process with
the correct privileges to directly read from or write to to another
process's address space.", which, if the reading process has somehow
gained privileges (as that never happens) is your security issue right
there.

So disable it - people who really need that normally know what they're
doing and also know how to enable it.

Signed-off-by: Borislav Petkov <[email protected]>
---
mm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 82fed4eb2b6f..3b6347cf4c06 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -352,7 +352,7 @@ endchoice
config CROSS_MEMORY_ATTACH
bool "Cross Memory Support"
depends on MMU
- default y
+ default n
help
Enabling this option adds the system calls process_vm_readv and
process_vm_writev which allow a process with the correct privileges
--
1.7.11.rc1


--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

2012-06-05 16:47:12

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] CMA: Do no enable it by default

On Tue, Jun 5, 2012 at 4:05 AM, Borislav Petkov <[email protected]> wrote:
>
> Besides, in the config option it says "... which allow a process with
> the correct privileges to directly read from or write to to another
> process's address space.", which, if the reading process has somehow
> gained privileges (as that never happens) is your security issue right
> there.

What?

It's using the same privileges as ptrace. If you are allowed to ptrace
somebody, there's no security issue.

Also, the reason it's "default y" is that the feature actually made it
in earlier (with no config option at all). Now, I certainly agree that
we could turn it off by default since it's not that common, but at the
same time none of your actual commit comments make sense, so that
would have to be fixed first.

Linus

2012-06-05 17:08:58

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] CMA: Do no enable it by default

On Tue, Jun 05, 2012 at 09:46:49AM -0700, Linus Torvalds wrote:
> On Tue, Jun 5, 2012 at 4:05 AM, Borislav Petkov <[email protected]> wrote:
> >
> > Besides, in the config option it says "... which allow a process with
> > the correct privileges to directly read from or write to to another
> > process's address space.", which, if the reading process has somehow
> > gained privileges (as that never happens) is your security issue right
> > there.
>
> What?
>
> It's using the same privileges as ptrace. If you are allowed to ptrace
> somebody, there's no security issue.

It didn't sound to me like that from the text - to my paranoid mind this
sounds like some process reading or writing some other process' address
space and changing stuff arbitrarily.

Maybe the text should be made more soothing so that no alarms go off
while reading it :-).

> Also, the reason it's "default y" is that the feature actually made it
> in earlier (with no config option at all).

I know, and it shouldn've been but it's too late now.

> Now, I certainly agree that we could turn it off by default since
> it's not that common, but at the same time none of your actual commit
> comments make sense, so that would have to be fixed first.

Sure, will do.

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

2012-06-05 18:03:32

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v2] CMA: Do no enable it by default

From: Borislav Petkov <[email protected]>

CROSS_MEMORY_ATTACH is a MPI feature which shouldn't be enabled by
default on every linux system simply because the majority of users do
not need it.

In the config option it says "... which allow a process with the correct
privileges to directly read from or write to to another process's
address space." but this is the normal ptrace case where if one process
has the required privileges, it can access another process' address
space.

So disable it - people who really need that normally know what they're
doing and also know how to enable it.

Signed-off-by: Borislav Petkov <[email protected]>
---

-v2: Correct commit message.

mm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 82fed4eb2b6f..3b6347cf4c06 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -352,7 +352,7 @@ endchoice
config CROSS_MEMORY_ATTACH
bool "Cross Memory Support"
depends on MMU
- default y
+ default n
help
Enabling this option adds the system calls process_vm_readv and
process_vm_writev which allow a process with the correct privileges
--
1.7.11.rc1

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

2012-06-06 01:25:10

by Christopher Yeoh

[permalink] [raw]
Subject: Re: [PATCH] CMA: Do no enable it by default

On Tue, 5 Jun 2012 13:05:41 +0200
Borislav Petkov <[email protected]> wrote:
>
> CROSS_MEMORY_ATTACH is a MPI feature which shouldn't be enabled by
> default on every linux system simply because the majority of users do
> not need it.

btw although CMA was primarily written for MPI it is used by more than
just MPI implementations. For example, recent versions of strace now
use it instead of PTRACE_PEEKDATA if its available:

http://article.gmane.org/gmane.comp.sysutils.strace.devel/2467/match=process_vm_readv

> Besides, in the config option it says "... which allow a process with
> the correct privileges to directly read from or write to to another
> process's address space.", which, if the reading process has somehow
> gained privileges (as that never happens) is your security issue right
> there.

The privileges required are exactly the same as required to ptrace the
target. You're rather stuffed anyway if you have a hostile process with
those privileges.

Regards,

Chris
--
[email protected]

2012-06-06 12:59:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] CMA: Do no enable it by default

On Wed, Jun 06, 2012 at 10:54:59AM +0930, Christopher Yeoh wrote:
> > CROSS_MEMORY_ATTACH is a MPI feature which shouldn't be enabled by
> > default on every linux system simply because the majority of users do
> > not need it.
>
> btw although CMA was primarily written for MPI it is used by more than
> just MPI implementations. For example, recent versions of strace now
> use it instead of PTRACE_PEEKDATA if its available:
>
> http://article.gmane.org/gmane.comp.sysutils.strace.devel/2467/match=process_vm_readv

I see.

Looks like process_vm_readv() is faster than PTRACE_PEEKDATA. You
could add this to the config option text so people can know why they
could/should enable CMA.

Oh, and also the note about ptrace privileges below :-).

> > Besides, in the config option it says "... which allow a process with
> > the correct privileges to directly read from or write to to another
> > process's address space.", which, if the reading process has somehow
> > gained privileges (as that never happens) is your security issue right
> > there.
>
> The privileges required are exactly the same as required to ptrace the
> target. You're rather stuffed anyway if you have a hostile process with
> those privileges.

Ok.

Thanks.

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551