2004-11-29 19:35:20

by Hugh Dickins

[permalink] [raw]
Subject: [PATCH] shmtcl SHM_LOCK perms

Michael Kerrisk has observed that at present any process can SHM_LOCK
any shm segment of size within process RLIMIT_MEMLOCK, despite having no
permissions on the segment: surprising, though not obviously evil. And
any process can SHM_UNLOCK any shm segment, despite no permissions on it:
that is surely wrong.

Unless CAP_IPC_LOCK, restrict both SHM_LOCK and SHM_UNLOCK to when the
process euid matches the shm owner or creator: that seems the least
surprising behaviour, which could be relaxed if a need appears later.

Signed-off-by: Hugh Dickins <[email protected]>

--- 2.6.10-rc2-bk13/ipc/shm.c 2004-11-15 16:21:23.000000000 +0000
+++ linux/ipc/shm.c 2004-11-29 18:07:06.398464576 +0000
@@ -511,11 +511,6 @@ asmlinkage long sys_shmctl (int shmid, i
case SHM_LOCK:
case SHM_UNLOCK:
{
- /* Allow superuser to lock segment in memory */
- if (!can_do_mlock() && cmd == SHM_LOCK) {
- err = -EPERM;
- goto out;
- }
shp = shm_lock(shmid);
if(shp==NULL) {
err = -EINVAL;
@@ -525,6 +520,16 @@ asmlinkage long sys_shmctl (int shmid, i
if(err)
goto out_unlock;

+ if (!capable(CAP_IPC_LOCK)) {
+ err = -EPERM;
+ if (current->euid != shp->shm_perm.uid &&
+ current->euid != shp->shm_perm.cuid)
+ goto out_unlock;
+ if (cmd == SHM_LOCK &&
+ !current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur)
+ goto out_unlock;
+ }
+
err = security_shm_shmctl(shp, cmd);
if (err)
goto out_unlock;


2004-11-30 20:55:26

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH] shmtcl SHM_LOCK perms

* Hugh Dickins ([email protected]) wrote:
> Michael Kerrisk has observed that at present any process can SHM_LOCK
> any shm segment of size within process RLIMIT_MEMLOCK, despite having no
> permissions on the segment: surprising, though not obviously evil. And
> any process can SHM_UNLOCK any shm segment, despite no permissions on it:
> that is surely wrong.

You may be neither the owner, nor the creator of a segment but have read
access to it. In which case you could simply copy the contents of the
segment anywhere you like, which has similar effect to SHM_UNLOCK from
the point of view of paging out sensitive data.

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

2004-12-01 01:04:43

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] shmtcl SHM_LOCK perms

On Tue, 30 Nov 2004, Chris Wright wrote:
> * Hugh Dickins ([email protected]) wrote:
> > Michael Kerrisk has observed that at present any process can SHM_LOCK
> > any shm segment of size within process RLIMIT_MEMLOCK, despite having no
> > permissions on the segment: surprising, though not obviously evil. And
> > any process can SHM_UNLOCK any shm segment, despite no permissions on it:
> > that is surely wrong.
>
> You may be neither the owner, nor the creator of a segment but have read
> access to it. In which case you could simply copy the contents of the
> segment anywhere you like, which has similar effect to SHM_UNLOCK from
> the point of view of paging out sensitive data.

True, and if securing sensitive data against pageout were the only reason
for SHM_LOCK, then I guess it might be an argument for letting anyone with
read permission do SHM_UNLOCK.

But that's not the only reason for SHM_LOCK, and all you're telling us
there is that the owner of sensitive data should be careful who they
give read permission to - indeed! So I still tend to agree with
Michael, that the most natural restriction is to owner or creator -
relax that if some app actually has a good case for relaxing it.

Hugh

2004-12-01 04:55:48

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH] shmtcl SHM_LOCK perms

* Hugh Dickins ([email protected]) wrote:
> But that's not the only reason for SHM_LOCK, and all you're telling us
> there is that the owner of sensitive data should be careful who they
> give read permission to - indeed! So I still tend to agree with
> Michael, that the most natural restriction is to owner or creator -
> relax that if some app actually has a good case for relaxing it.

Yup, I agree.

thanks,
-chris