From b78b6af66a5fbaf17d7e6bfc32384df5e34408c8 Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Wed, 12 Apr 2006 14:34:27 -0700 Subject: [PATCH] shmat: stop mprotect from giving write permission to a readonly attachment (CVE-2006-1524) I found that all of 2.4 and 2.6 have been letting mprotect give write permission to a readonly attachment of shared memory, whether or not IPC would give the caller that permission. SUS says "The behaviour of this function [mprotect] is unspecified if the mapping was not established by a call to mmap", but I don't think we can interpret that as allowing it to subvert IPC permissions. I haven't tried 2.2, but the 2.2.26 source looks like it gets it right; and the patch below reproduces that behaviour - mprotect cannot be used to add write permission to a shared memory segment attached readonly. This patch is simple, and I'm sure it's what we should have done in 2.4.0: if you want to go on to switch write permission on and off with mprotect, just don't attach the segment readonly in the first place. However, we could have accumulated apps which attach readonly (even though they would be permitted to attach read/write), and which subsequently use mprotect to switch write permission on and off: it's not unreasonable. I was going to add a second ipcperms check in do_shmat, to check for writable when readonly, and if not writable find_vma and clear VM_MAYWRITE. But security_ipc_permission might do auditing, and it seems wrong to report an attempt for write permission when there has been none. Or we could flag the vma as SHM, note the shmid or shp in vm_private_data, and then get mprotect to check. But the patch below is a lot simpler: I'd rather stick with it, if we can convince ourselves somehow that it'll be safe. Signed-off-by: Hugh Dickins Signed-off-by: Andrew Morton Signed-off-by: Greg Kroah-Hartman --- ipc/shm.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'ipc') diff --git a/ipc/shm.c b/ipc/shm.c index 6b0c9af5bbf7..1c2faf62bc73 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -162,6 +162,8 @@ static int shm_mmap(struct file * file, struct vm_area_struct * vma) ret = shmem_mmap(file, vma); if (ret == 0) { vma->vm_ops = &shm_vm_ops; + if (!(vma->vm_flags & VM_WRITE)) + vma->vm_flags &= ~VM_MAYWRITE; shm_inc(file->f_dentry->d_inode->i_ino); } -- cgit v1.2.3 From a9a5cd5d2a57fb76dbae2115450f777b69beccf7 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Mon, 17 Apr 2006 15:39:23 +0400 Subject: [PATCH] IPC: access to unmapped vmalloc area in grow_ary() grow_ary() should not copy struct ipc_id_ary (it copies new->p, not new). Due to this, memcpy() src pointer could hit unmapped vmalloc page when near page boundary. Found during OpenVZ stress testing Signed-off-by: Alexey Kuznetsov Signed-off-by: Kirill Korotaev Signed-off-by: Linus Torvalds --- ipc/util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'ipc') diff --git a/ipc/util.c b/ipc/util.c index 5e785a29e1e6..b3dcfad3b4f7 100644 --- a/ipc/util.c +++ b/ipc/util.c @@ -183,8 +183,7 @@ static int grow_ary(struct ipc_ids* ids, int newsize) if(new == NULL) return size; new->size = newsize; - memcpy(new->p, ids->entries->p, sizeof(struct kern_ipc_perm *)*size + - sizeof(struct ipc_id_ary)); + memcpy(new->p, ids->entries->p, sizeof(struct kern_ipc_perm *)*size); for(i=size;ip[i] = NULL; } -- cgit v1.2.3 From 073115d6b29c7910feaa08241c6484637f5ca958 Mon Sep 17 00:00:00 2001 From: Steve Grubb Date: Sun, 2 Apr 2006 17:07:33 -0400 Subject: [PATCH] Rework of IPC auditing 1) The audit_ipc_perms() function has been split into two different functions: - audit_ipc_obj() - audit_ipc_set_perm() There's a key shift here... The audit_ipc_obj() collects the uid, gid, mode, and SElinux context label of the current ipc object. This audit_ipc_obj() hook is now found in several places. Most notably, it is hooked in ipcperms(), which is called in various places around the ipc code permforming a MAC check. Additionally there are several places where *checkid() is used to validate that an operation is being performed on a valid object while not necessarily having a nearby ipcperms() call. In these locations, audit_ipc_obj() is called to ensure that the information is captured by the audit system. The audit_set_new_perm() function is called any time the permissions on the ipc object changes. In this case, the NEW permissions are recorded (and note that an audit_ipc_obj() call exists just a few lines before each instance). 2) Support for an AUDIT_IPC_SET_PERM audit message type. This allows for separate auxiliary audit records for normal operations on an IPC object and permissions changes. Note that the same struct audit_aux_data_ipcctl is used and populated, however there are separate audit_log_format statements based on the type of the message. Finally, the AUDIT_IPC block of code in audit_free_aux() was extended to handle aux messages of this new type. No more mem leaks I hope ;-) Signed-off-by: Al Viro --- include/linux/audit.h | 7 +++++-- ipc/msg.c | 11 ++++++++++- ipc/sem.c | 11 ++++++++++- ipc/shm.c | 19 +++++++++++++++--- ipc/util.c | 7 ++++++- kernel/auditsc.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 98 insertions(+), 11 deletions(-) (limited to 'ipc') diff --git a/include/linux/audit.h b/include/linux/audit.h index d5c40823e166..b74c148f14e3 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -83,6 +83,7 @@ #define AUDIT_CONFIG_CHANGE 1305 /* Audit system configuration change */ #define AUDIT_SOCKADDR 1306 /* sockaddr copied as syscall arg */ #define AUDIT_CWD 1307 /* Current working directory */ +#define AUDIT_IPC_SET_PERM 1311 /* IPC new permissions record type */ #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */ #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */ @@ -319,7 +320,8 @@ extern void auditsc_get_stamp(struct audit_context *ctx, struct timespec *t, unsigned int *serial); extern int audit_set_loginuid(struct task_struct *task, uid_t loginuid); extern uid_t audit_get_loginuid(struct audit_context *ctx); -extern int audit_ipc_perms(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm *ipcp); +extern int audit_ipc_obj(struct kern_ipc_perm *ipcp); +extern int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm *ipcp); extern int audit_socketcall(int nargs, unsigned long *args); extern int audit_sockaddr(int len, void *addr); extern int audit_avc_path(struct dentry *dentry, struct vfsmount *mnt); @@ -338,7 +340,8 @@ extern int audit_set_macxattr(const char *name); #define audit_inode_child(d,i,p) do { ; } while (0) #define auditsc_get_stamp(c,t,s) do { BUG(); } while (0) #define audit_get_loginuid(c) ({ -1; }) -#define audit_ipc_perms(q,u,g,m,i) ({ 0; }) +#define audit_ipc_obj(i) ({ 0; }) +#define audit_ipc_set_perm(q,u,g,m,i) ({ 0; }) #define audit_socketcall(n,a) ({ 0; }) #define audit_sockaddr(len, addr) ({ 0; }) #define audit_avc_path(dentry, mnt) ({ 0; }) diff --git a/ipc/msg.c b/ipc/msg.c index 48a7f17a7236..7d1340ccb16b 100644 --- a/ipc/msg.c +++ b/ipc/msg.c @@ -13,6 +13,9 @@ * mostly rewritten, threaded and wake-one semantics added * MSGMAX limit removed, sysctl's added * (c) 1999 Manfred Spraul + * + * support for audit of ipc object properties and permission changes + * Dustin Kirkland */ #include @@ -447,6 +450,11 @@ asmlinkage long sys_msgctl (int msqid, int cmd, struct msqid_ds __user *buf) if (msg_checkid(msq,msqid)) goto out_unlock_up; ipcp = &msq->q_perm; + + err = audit_ipc_obj(ipcp); + if (err) + goto out_unlock_up; + err = -EPERM; if (current->euid != ipcp->cuid && current->euid != ipcp->uid && !capable(CAP_SYS_ADMIN)) @@ -460,7 +468,8 @@ asmlinkage long sys_msgctl (int msqid, int cmd, struct msqid_ds __user *buf) switch (cmd) { case IPC_SET: { - if ((err = audit_ipc_perms(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode, ipcp))) + err = audit_ipc_set_perm(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode, ipcp); + if (err) goto out_unlock_up; err = -EPERM; diff --git a/ipc/sem.c b/ipc/sem.c index 642659cd596b..7919f8ece6ba 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -61,6 +61,9 @@ * (c) 2001 Red Hat Inc * Lockless wakeup * (c) 2003 Manfred Spraul + * + * support for audit of ipc object properties and permission changes + * Dustin Kirkland */ #include @@ -820,6 +823,11 @@ static int semctl_down(int semid, int semnum, int cmd, int version, union semun goto out_unlock; } ipcp = &sma->sem_perm; + + err = audit_ipc_obj(ipcp); + if (err) + goto out_unlock; + if (current->euid != ipcp->cuid && current->euid != ipcp->uid && !capable(CAP_SYS_ADMIN)) { err=-EPERM; @@ -836,7 +844,8 @@ static int semctl_down(int semid, int semnum, int cmd, int version, union semun err = 0; break; case IPC_SET: - if ((err = audit_ipc_perms(0, setbuf.uid, setbuf.gid, setbuf.mode, ipcp))) + err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode, ipcp); + if (err) goto out_unlock; ipcp->uid = setbuf.uid; ipcp->gid = setbuf.gid; diff --git a/ipc/shm.c b/ipc/shm.c index 1c2faf62bc73..809896851902 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -13,6 +13,8 @@ * Shared /dev/zero support, Kanoj Sarcar * Move the mm functionality over to mm/shmem.c, Christoph Rohland * + * support for audit of ipc object properties and permission changes + * Dustin Kirkland */ #include @@ -542,6 +544,10 @@ asmlinkage long sys_shmctl (int shmid, int cmd, struct shmid_ds __user *buf) if(err) goto out_unlock; + err = audit_ipc_obj(&(shp->shm_perm)); + if (err) + goto out_unlock; + if (!capable(CAP_IPC_LOCK)) { err = -EPERM; if (current->euid != shp->shm_perm.uid && @@ -594,6 +600,10 @@ asmlinkage long sys_shmctl (int shmid, int cmd, struct shmid_ds __user *buf) if(err) goto out_unlock_up; + err = audit_ipc_obj(&(shp->shm_perm)); + if (err) + goto out_unlock_up; + if (current->euid != shp->shm_perm.uid && current->euid != shp->shm_perm.cuid && !capable(CAP_SYS_ADMIN)) { @@ -627,12 +637,15 @@ asmlinkage long sys_shmctl (int shmid, int cmd, struct shmid_ds __user *buf) err=-EINVAL; if(shp==NULL) goto out_up; - if ((err = audit_ipc_perms(0, setbuf.uid, setbuf.gid, - setbuf.mode, &(shp->shm_perm)))) - goto out_unlock_up; err = shm_checkid(shp,shmid); if(err) goto out_unlock_up; + err = audit_ipc_obj(&(shp->shm_perm)); + if (err) + goto out_unlock_up; + err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode, &(shp->shm_perm)); + if (err) + goto out_unlock_up; err=-EPERM; if (current->euid != shp->shm_perm.uid && current->euid != shp->shm_perm.cuid && diff --git a/ipc/util.c b/ipc/util.c index b3dcfad3b4f7..8193299f45f6 100644 --- a/ipc/util.c +++ b/ipc/util.c @@ -10,6 +10,8 @@ * Manfred Spraul * Oct 2002 - One lock per IPC id. RCU ipc_free for lock-free grow_ary(). * Mingming Cao + * Mar 2006 - support for audit of ipc object properties + * Dustin Kirkland */ #include @@ -27,6 +29,7 @@ #include #include #include +#include #include @@ -464,8 +467,10 @@ void ipc_rcu_putref(void *ptr) int ipcperms (struct kern_ipc_perm *ipcp, short flag) { /* flag will most probably be 0 or S_...UGO from */ - int requested_mode, granted_mode; + int requested_mode, granted_mode, err; + if (unlikely((err = audit_ipc_obj(ipcp)))) + return err; requested_mode = (flag >> 6) | (flag >> 3) | flag; granted_mode = ipcp->mode; if (current->euid == ipcp->cuid || current->euid == ipcp->uid) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index d94e0404113c..a300736ee037 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -646,6 +646,25 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts } break; } + case AUDIT_IPC_SET_PERM: { + struct audit_aux_data_ipcctl *axi = (void *)aux; + audit_log_format(ab, + " new qbytes=%lx new iuid=%u new igid=%u new mode=%x", + axi->qbytes, axi->uid, axi->gid, axi->mode); + if (axi->osid != 0) { + char *ctx = NULL; + u32 len; + if (selinux_ctxid_to_string( + axi->osid, &ctx, &len)) { + audit_log_format(ab, " osid=%u", + axi->osid); + call_panic = 1; + } else + audit_log_format(ab, " obj=%s", ctx); + kfree(ctx); + } + break; } + case AUDIT_SOCKETCALL: { int i; struct audit_aux_data_socketcall *axs = (void *)aux; @@ -1148,7 +1167,36 @@ uid_t audit_get_loginuid(struct audit_context *ctx) } /** - * audit_ipc_perms - record audit data for ipc + * audit_ipc_obj - record audit data for ipc object + * @ipcp: ipc permissions + * + * Returns 0 for success or NULL context or < 0 on error. + */ +int audit_ipc_obj(struct kern_ipc_perm *ipcp) +{ + struct audit_aux_data_ipcctl *ax; + struct audit_context *context = current->audit_context; + + if (likely(!context)) + return 0; + + ax = kmalloc(sizeof(*ax), GFP_ATOMIC); + if (!ax) + return -ENOMEM; + + ax->uid = ipcp->uid; + ax->gid = ipcp->gid; + ax->mode = ipcp->mode; + selinux_get_ipc_sid(ipcp, &ax->osid); + + ax->d.type = AUDIT_IPC; + ax->d.next = context->aux; + context->aux = (void *)ax; + return 0; +} + +/** + * audit_ipc_set_perm - record audit data for new ipc permissions * @qbytes: msgq bytes * @uid: msgq user id * @gid: msgq group id @@ -1156,7 +1204,7 @@ uid_t audit_get_loginuid(struct audit_context *ctx) * * Returns 0 for success or NULL context or < 0 on error. */ -int audit_ipc_perms(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm *ipcp) +int audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, struct kern_ipc_perm *ipcp) { struct audit_aux_data_ipcctl *ax; struct audit_context *context = current->audit_context; @@ -1174,7 +1222,7 @@ int audit_ipc_perms(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode, str ax->mode = mode; selinux_get_ipc_sid(ipcp, &ax->osid); - ax->d.type = AUDIT_IPC; + ax->d.type = AUDIT_IPC_SET_PERM; ax->d.next = context->aux; context->aux = (void *)ax; return 0; -- cgit v1.2.3