[PATCH] shmtcl SHM_LOCK perms

[PATCH] shmtcl SHM_LOCK perms

[Hugh Dickins]

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 <hugh@veritas.com>

--- 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;

Re: [PATCH] shmtcl SHM_LOCK perms

[Chris Wright]

* Hugh Dickins (hugh@veritas.com) 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

Re: [PATCH] shmtcl SHM_LOCK perms

[Hugh Dickins]

On Tue, 30 Nov 2004, Chris Wright wrote:
> * Hugh Dickins (hugh@veritas.com) 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

Re: [PATCH] shmtcl SHM_LOCK perms

[Chris Wright]

* Hugh Dickins (hugh@veritas.com) 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