[PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

In drivers/char/mem.c there's open_port(), which is used as open_mem()
and open_kmem() as well. I don't see the benefit of this, since
/dev/mem and /dev/kmem are already protected by filesystem
permissions.

mem.c, line 526:
static int open_port(struct inode * inode, struct file * filp)
{
	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}

If anyone knows, why this is done this way, please let me
know. Otherwise, I suggest the patch below.

Regards, Olaf.

--- a/drivers/char/mem.c	Sat Oct  5 18:44:55 2002
+++ b/drivers/char/mem.c	Sun Oct 13 13:59:25 2002
@@ -533,15 +533,12 @@
 #define full_lseek      null_lseek
 #define write_zero	write_null
 #define read_full       read_zero
-#define open_mem	open_port
-#define open_kmem	open_mem
 
 static struct file_operations mem_fops = {
 	llseek:		memory_lseek,
 	read:		read_mem,
 	write:		write_mem,
 	mmap:		mmap_mem,
-	open:		open_mem,
 };
 
 static struct file_operations kmem_fops = {
@@ -549,7 +546,6 @@
 	read:		read_kmem,
 	write:		write_kmem,
 	mmap:		mmap_kmem,
-	open:		open_kmem,
 };
 
 static struct file_operations null_fops = {

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Olaf Dietsche <olaf.dietsche#list.linux-kernel@t-online.de> writes:

> In drivers/char/mem.c there's open_port(), which is used as open_mem()
> and open_kmem() as well. I don't see the benefit of this, since
> /dev/mem and /dev/kmem are already protected by filesystem
> permissions.
>
> mem.c, line 526:
> static int open_port(struct inode * inode, struct file * filp)
> {
> 	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
> }
>
> If anyone knows, why this is done this way, please let me
> know. Otherwise, I suggest the patch below.

I haven't got a convincing answer against this patch, so far. The
patch applies to 2.5.43 as well.
Linus, please apply.

Regards, Olaf.

--- a/drivers/char/mem.c	Sat Oct  5 18:44:55 2002
+++ b/drivers/char/mem.c	Sun Oct 13 13:59:25 2002
@@ -533,15 +533,12 @@
 #define full_lseek      null_lseek
 #define write_zero	write_null
 #define read_full       read_zero
-#define open_mem	open_port
-#define open_kmem	open_mem
 
 static struct file_operations mem_fops = {
 	llseek:		memory_lseek,
 	read:		read_mem,
 	write:		write_mem,
 	mmap:		mmap_mem,
-	open:		open_mem,
 };
 
 static struct file_operations kmem_fops = {
@@ -549,7 +546,6 @@
 	read:		read_kmem,
 	write:		write_kmem,
 	mmap:		mmap_kmem,
-	open:		open_kmem,
 };
 
 static struct file_operations null_fops = {

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Chris Evans]

Hi,

No, please DON'T apply.

If /dev/mem and /dev/kmem are only protected by
filesystem permissions, then:

1) Ownership of either will confer CAP_SYS_RAWIO
2) CAP_DAC_OVERRIDE, etc. will confer CAP_SYS_RAWIO
3) Ability to mknod() one of the these will confer
CAP_SYS_RAWIO (I think to make one you only need
CAP_SYS_ADMIN, could be wrong).

CAP_SYS_RAWIO is extremely dangerous and must not
be leverageable via other system capabilities or
filesystem permissions.

There are many people who have systems with elaborate
security setups where root access is limited in what
it can do. If we allow a root user to get
CAP_SYS_RAWIO easily, this is lost.

Cheers
Chris

Quoting Olaf Dietsche
<olaf.dietsche#list.linux-kernel@t-online.de>:

> Olaf Dietsche
<olaf.dietsche#list.linux-kernel@t-online.de> writes:
> 
> > In drivers/char/mem.c there's open_port(), which is
used as open_mem()
> > and open_kmem() as well. I don't see the benefit of
this, since
> > /dev/mem and /dev/kmem are already protected by
filesystem
> > permissions.
> >
> > mem.c, line 526:
> > static int open_port(struct inode * inode, struct
file * filp)
> > {
> > 	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
> > }
> >
> > If anyone knows, why this is done this way, please
let me
> > know. Otherwise, I suggest the patch below.
> 
> I haven't got a convincing answer against this patch,
so far. The
> patch applies to 2.5.43 as well.
> Linus, please apply.
> 
> Regards, Olaf.
> 
> --- a/drivers/char/mem.c	Sat Oct  5 18:44:55 2002
> +++ b/drivers/char/mem.c	Sun Oct 13 13:59:25 2002
> @@ -533,15 +533,12 @@
>  #define full_lseek      null_lseek
>  #define write_zero	write_null
>  #define read_full       read_zero
> -#define open_mem	open_port
> -#define open_kmem	open_mem
>  
>  static struct file_operations mem_fops = {
>  	llseek:		memory_lseek,
>  	read:		read_mem,
>  	write:		write_mem,
>  	mmap:		mmap_mem,
> -	open:		open_mem,
>  };
>  
>  static struct file_operations kmem_fops = {
> @@ -549,7 +546,6 @@
>  	read:		read_kmem,
>  	write:		write_kmem,
>  	mmap:		mmap_kmem,
> -	open:		open_kmem,
>  };
>  
>  static struct file_operations null_fops = {
> -
> To unsubscribe from this list: send the line
"unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at 
http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Chris Wright]

* Olaf Dietsche (olaf.dietsche#list.linux-kernel@t-online.de) wrote:
> Olaf Dietsche <olaf.dietsche#list.linux-kernel@t-online.de> writes:
> 
> > In drivers/char/mem.c there's open_port(), which is used as open_mem()
> > and open_kmem() as well. I don't see the benefit of this, since
> > /dev/mem and /dev/kmem are already protected by filesystem
> > permissions.
> >
> > mem.c, line 526:
> > static int open_port(struct inode * inode, struct file * filp)
> > {
> > 	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
> > }
> >
> > If anyone knows, why this is done this way, please let me
> > know. Otherwise, I suggest the patch below.
> 
> I haven't got a convincing answer against this patch, so far. The
> patch applies to 2.5.43 as well.
> Linus, please apply.

No way.  This is clearly a bad idea.  CAP_SYS_RAWIO should be treated very
seriously, look at what it enables.  CAP_DAC_OVERRIDE is substantially
less powerful, and if you remove this check, it would be the only
capability protecting this.

-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Chris Wright <chris@wirex.com> writes:

> * Olaf Dietsche (olaf.dietsche#list.linux-kernel@t-online.de) wrote:
>> 
>> I haven't got a convincing answer against this patch, so far. The
>> patch applies to 2.5.43 as well.
>> Linus, please apply.

Hehe, "please apply" is watched a lot more closely, it seems. Good to
know ;-)

> No way.  This is clearly a bad idea.  CAP_SYS_RAWIO should be treated very
> seriously, look at what it enables.  CAP_DAC_OVERRIDE is substantially
> less powerful, and if you remove this check, it would be the only
> capability protecting this.

$ grep -r CAP_SYS_RAWIO v2.5.43 | wc
55

Well, since CAP_SYS_RAWIO is such a dangerous beast and /dev/kmem can't
live without, then something like CAP_SYS_KMEM would be more
appropriate.

Here's a new untested patch against 2.5.43. Comments?

Regards, Olaf.

diff -urN a/drivers/char/mem.c b/drivers/char/mem.c
--- a/drivers/char/mem.c	Sat Oct  5 18:44:55 2002
+++ b/drivers/char/mem.c	Thu Oct 17 16:02:56 2002
@@ -525,7 +525,7 @@
 
 static int open_port(struct inode * inode, struct file * filp)
 {
-	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
+	return capable(CAP_SYS_KMEM) ? 0 : -EPERM;
 }
 
 #define mmap_kmem	mmap_mem
diff -urN a/include/linux/capability.h b/include/linux/capability.h
--- a/include/linux/capability.h	Sat Oct  5 18:43:38 2002
+++ b/include/linux/capability.h	Thu Oct 17 16:02:35 2002
@@ -283,6 +283,10 @@
 
 #define CAP_LEASE            28
 
+/* Allow access to system memory */
+
+#define CAP_SYS_KMEM         29
+
 #ifdef __KERNEL__
 /* 
  * Bounding set

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Oliver Neukum <oliver@neukum.name> writes:

>> diff -urN a/drivers/char/mem.c b/drivers/char/mem.c
>> --- a/drivers/char/mem.c	Sat Oct  5 18:44:55 2002
>> +++ b/drivers/char/mem.c	Thu Oct 17 16:02:56 2002
>> @@ -525,7 +525,7 @@
>>
>>  static int open_port(struct inode * inode, struct file * filp)
>>  {
>> -	return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
>> +	return capable(CAP_SYS_KMEM) ? 0 : -EPERM;
>
> return capable(CAP_SYS_KMEM) && capable(CAP_SYS_RAWIO) ? 0 : _EPERM;
>
> Unless you check for RAWIO you can gain RAWIO by illegitimate means.

It's embarrassing, but it took until now for me to realize, that this
tries to protect CAP_SYS_RAWIO and not /dev/kmem. Well, thanks for
being patient with me.

> Now whether one place justifies a whole capability is another question.

This is unnecessary then.

Regards, Olaf.

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Manfred Spraul <manfred@colorfullife.com> writes:

>> In drivers/char/mem.c there's open_port(), which is used as open_mem()
>> and open_kmem() as well. I don't see the benefit of this, since
>> /dev/mem and /dev/kmem are already protected by filesystem
>> permissions.
>>
> capabilities can be stricter than filesystem permissions

Which means, it prevents me from giving access to /dev/kmem to an
otherwise unprivileged process.

> , and the call
> is needed to update the PF_SUPERPRIV process flag.

What exactly is PF_SUPERPRIV good for? I see no real use in the
source. There is exactly one test for this flag (kernel/acct.c:336),
then sets another flag (ASU), which in turn is used nowhere else.

So, I think we could get rid of this flag as well. Comments?

Regards, Olaf.

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Manfred Spraul <manfred@colorfullife.com> writes:

> Olaf Dietsche wrote:
>  > Manfred Spraul <manfred@colorfullife.com> writes:
>  >
>  >
>  >>>In drivers/char/mem.c there's open_port(), which is used as open_mem()
>  >>>and open_kmem() as well. I don't see the benefit of this, since
>  >>>/dev/mem and /dev/kmem are already protected by filesystem
>  >>>permissions.
>  >>>
>  >>
>  >>capabilities can be stricter than filesystem permissions
>  >
>  >
>  > Which means, it prevents me from giving access to /dev/kmem to an
>  > otherwise unprivileged process.
>  >
> Do you know what access to /dev/kmem means?

Which is not the point. Just assume for a moment, I know what I'm
doing :-)

> A process that can read /dev/kmem can read /etc/shadow and probaly all
> other files he can force into memory.
>
> A process that can write to /dev/kmem can give himself ultimate
> capabilities by modifying it's own uid/capability set.

Now, I have to run this process as root, regardless of filesystem
permissions. So, if I trust this particular process with full
privileges now, there's no problem in reducing its power a little bit.

> Have you tried to disable the capabilities? I think there is a kernel
> option that disables them.

I don't want to disable capabilities. I want to get rid of this
particular use.

>>>, and the call
>>>is needed to update the PF_SUPERPRIV process flag.
>> What exactly is PF_SUPERPRIV good for? I see no real use in the
>> source. There is exactly one test for this flag (kernel/acct.c:336),
>> then sets another flag (ASU), which in turn is used nowhere else.
>> So, I think we could get rid of this flag as well. Comments?
>>
>
> Part of BSD process accounting - you have just broken backward
> compatibility with user space.

Ok, thanks for this hint.

Regards, Olaf.

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Manfred Spraul]

Olaf Dietsche wrote:
> 
> Now, I have to run this process as root, regardless of filesystem
> permissions. So, if I trust this particular process with full
> privileges now, there's no problem in reducing its power a little bit.
> 
What about writing a small wrapper application that drops all priveleges 
except CAP_RAWIO, switches to user to the user you want, then execs the 
target application that needs to access /dev/kmem?

Or store the capabilities in the filesystem, but I don't know which 
filesystem supports that.

--
	Manfred

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Olaf Dietsche]

Manfred Spraul <manfred@colorfullife.com> writes:

> Olaf Dietsche wrote:
>> Now, I have to run this process as root, regardless of filesystem
>> permissions. So, if I trust this particular process with full
>> privileges now, there's no problem in reducing its power a little bit.
>>
> What about writing a small wrapper application that drops all
> priveleges except CAP_RAWIO, switches to user to the user you want,
> then execs the target application that needs to access /dev/kmem?

I just tried this, but I didn't succeed. :-(

> Or store the capabilities in the filesystem, but I don't know which
> filesystem supports that.

There's none so far.

Regards, Olaf.

Re: [PATCH][RFC] 2.5.42: remove capable(CAP_SYS_RAWIO) check from open_kmem

[Andreas Steinmetz]

>>What about writing a small wrapper application that drops all
>>priveleges except CAP_RAWIO, switches to user to the user you want,
>>then execs the target application that needs to access /dev/kmem?
> 
> 
> I just tried this, but I didn't succeed. :-(
> 
> 
>>Or store the capabilities in the filesystem, but I don't know which
>>filesystem supports that.
> 
> 
> There's none so far.
> 

Not exactly. Well, not really a filesystem. But there's already security 
use of this feature you want to remove. Think LSM. Look at e.g. LIDS. Im 
using this additional protection already under 2.4.x to prevent uid 0 
processes to access /dev/mem and /dev/kmem where not explicitely 
granted. Please, _don't_ remove the capability check because you don't 
see any use for it as there _is_ already use for it.

-- 
Andreas Steinmetz
D.O.M. Datenverarbeitung GmbH