[PATCH] trivial: make mmap_min_addr perms 600

[PATCH] trivial: make mmap_min_addr perms 600

[Kyle McMartin]

Chuck points out that mmap_min_addr is 644...

CAP_SYS_RAWIO will deny users read/write to the file, let's let them see
that this is intended.

Signed-off-by: Kyle McMartin <kyle@redhat.com>
---
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8686b0f..5868481 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1209,7 +1209,7 @@ static struct ctl_table vm_table[] = {
 		.procname	= "mmap_min_addr",
 		.data		= &dac_mmap_min_addr,
 		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
+		.mode		= 0600,
 		.proc_handler	= mmap_min_addr_handler,
 	},
 #endif

Re: [PATCH] trivial: make mmap_min_addr perms 600

[Eric Paris]

On Wed, 2010-04-21 at 10:00 -0400, Kyle McMartin wrote:
> Chuck points out that mmap_min_addr is 644...
> 
> CAP_SYS_RAWIO will deny users read/write to the file, let's let them see
> that this is intended.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

I'm fine with it.  RAWIO was only really added to block writes as I
recall, but I don't see a good reason normal users need to see this and
blocking them with rwx perms first is a good idea.

Acked-by: Eric Paris <eparis@redhat.com>

James, do you want to pick up and push towards linus?

-Eric
> ---
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 8686b0f..5868481 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1209,7 +1209,7 @@ static struct ctl_table vm_table[] = {
>  		.procname	= "mmap_min_addr",
>  		.data		= &dac_mmap_min_addr,
>  		.maxlen		= sizeof(unsigned long),
> -		.mode		= 0644,
> +		.mode		= 0600,
>  		.proc_handler	= mmap_min_addr_handler,
>  	},
>  #endif

Re: [PATCH] trivial: make mmap_min_addr perms 600

[Kees Cook]

On Wed, Apr 21, 2010 at 10:00:33AM -0400, Kyle McMartin wrote:
> Chuck points out that mmap_min_addr is 644...
> CAP_SYS_RAWIO will deny users read/write to the file, let's let them see
> that this is intended.

Hmm.  Denying read is actually an unintended side-effect of the
CAP_SYS_RAWIO check.

The value is useful information for an admin to see without being root
("is my system vulnerable to kernel NULL pointer attacks?") and its
setting is trivially easy for an attacker to determine by calling mmap()
in PAGE_SIZE increments starting at 0.

I would opt for greater transparency for the admin, and leave it 644 and
instead more carefully check CAP_SYS_RAWIO.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 security/min_addr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/security/min_addr.c b/security/min_addr.c
index e86f297..f728728 100644
--- a/security/min_addr.c
+++ b/security/min_addr.c
@@ -33,7 +33,7 @@ int mmap_min_addr_handler(struct ctl_table *table, int write,
 {
 	int ret;
 
-	if (!capable(CAP_SYS_RAWIO))
+	if (write && !capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
 	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-- 
1.7.0.4


-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH] trivial: make mmap_min_addr perms 600

[Kyle McMartin]

On Wed, Apr 21, 2010 at 11:29:11AM -0700, Kees Cook wrote:
> I would opt for greater transparency for the admin, and leave it 644 and
> instead more carefully check CAP_SYS_RAWIO.
> 
> Signed-off-by: Kees Cook <kees.cook@canonical.com>

fine with me.