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 = {
Manfred Spraul <[email protected]> writes:
> Olaf Dietsche wrote:
> > Manfred Spraul <[email protected]> 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.
Manfred Spraul <[email protected]> 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.
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
Manfred Spraul <[email protected]> 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.
Olaf Dietsche <olaf.dietsche#[email protected]> 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 = {
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#[email protected]>:
> Olaf Dietsche
<olaf.dietsche#[email protected]> 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 [email protected]
> More majordomo info at
http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
* Olaf Dietsche (olaf.dietsche#[email protected]) wrote:
> Olaf Dietsche <olaf.dietsche#[email protected]> 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
>>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
Chris Wright <[email protected]> writes:
> * Olaf Dietsche (olaf.dietsche#[email protected]) 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
Oliver Neukum <[email protected]> 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.