[PATCH] IPC_SET_PERM cleanup
Dustin Kirkland
dustin.kirkland at gmail.com
Mon May 8 18:29:49 UTC 2006
On 5/5/06, Linda Knippers <linda.knippers at hp.com> wrote:
> The following patch addresses most of the issues with the IPC_SET_PERM
> records as described in:
> https://www.redhat.com/archives/linux-audit/2006-May/msg00010.html
Hi Linda-
I apologize for the delay in my response. I'm pretty much permanently
away from Audit-related work, and just now got a chance to respond to
this.
First, let me point you to a thread wherein I explained why I made the
audit ipc changes that I did. It starts here:
https://www.redhat.com/archives/linux-audit/2006-March/msg00088.html
That said, thanks for testing some of this out more thoroughly and
posting your findings.
> To summarize, I made the following changes:
>
> 1. Changed sys_msgctl() and semctl_down() so that an IPC_SET_PERM
> record is emitted in the failure case as well as the success case.
> This matches the behavior in sys_shmctl(). I could simplify the
> code in sys_msgctl() and semctl_down() slightly but it would mean
> that in some error cases we could get an IPC_SET_PERM record
> without an IPC record and that seemed odd.
>
> 2. No change to the IPC record type, given no feedback on the backward
> compatibility question.
>
> 3. Removed the qbytes field from the IPC record. It wasn't being
> set and when audit_ipc_obj() is called from ipcperms(), the
> information isn't available. If we want the information in the IPC
> record, more extensive changes will be necessary. Since it only
> applies to message queues and it isn't really permission related, it
> doesn't seem worth it.
>
> 4. Removed the obj field from the IPC_SET_PERM record. This means that
> the kern_ipc_perm argument is no longer needed.
>
> 5. Replaced the spaces in the IPC_SET_PERM field names with underscores.
Thanks, that was my oversight. There should definitely be underscores
rather than spaces.
> I tested this with the lspp.22 kernel on an x86_64 box. Please let me
> know if you see any issues.
>
> -- ljk
>
> include/linux/audit.h | 2 +-
> ipc/msg.c | 9 +++++----
> ipc/sem.c | 8 +++++---
> ipc/shm.c | 2 +-
> kernel/auditsc.c | 22 +++++-----------------
> 5 files changed, 17 insertions(+), 26 deletions(-)
>
> --- linux-2.6.16.x86_64.orig/kernel/auditsc.c 2006-05-05 14:29:42.000000000 -0400
> +++ linux-2.6.16.x86_64/kernel/auditsc.c 2006-05-05 14:48:44.000000000 -0400
> @@ -665,8 +665,8 @@ static void audit_log_exit(struct audit_
> case AUDIT_IPC: {
> struct audit_aux_data_ipcctl *axi = (void *)aux;
> audit_log_format(ab,
> - " qbytes=%lx iuid=%u igid=%u mode=%x",
> - axi->qbytes, axi->uid, axi->gid, axi->mode);
> + "iuid=%u igid=%u mode=%x",
> + axi->uid, axi->gid, axi->mode);
> if (axi->osid != 0) {
> char *ctx = NULL;
> u32 len;
> @@ -684,21 +684,10 @@ static void audit_log_exit(struct audit_
> 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",
> + "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_EXECVE: {
> struct audit_aux_data_execve *axi = (void *)aux;
> int i;
> @@ -1232,7 +1221,7 @@ int audit_ipc_obj(struct kern_ipc_perm *
> *
> * Returns 0 for success or NULL context or < 0 on error.
> */
> -int audit_ipc_set_perm(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 audit_aux_data_ipcctl *ax;
> struct audit_context *context = current->audit_context;
> @@ -1248,7 +1237,6 @@ int audit_ipc_set_perm(unsigned long qby
> ax->uid = uid;
> ax->gid = gid;
> ax->mode = mode;
> - selinux_get_ipc_sid(ipcp, &ax->osid);
>
> ax->d.type = AUDIT_IPC_SET_PERM;
> ax->d.next = context->aux;
> --- linux-2.6.16.x86_64.orig/ipc/msg.c 2006-05-05 14:30:15.000000000 -0400
> +++ linux-2.6.16.x86_64/ipc/msg.c 2006-05-05 14:50:32.000000000 -0400
> @@ -454,6 +454,11 @@ asmlinkage long sys_msgctl (int msqid, i
> err = audit_ipc_obj(ipcp);
> if (err)
> goto out_unlock_up;
> + if (cmd==IPC_SET) {
> + err = audit_ipc_set_perm(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode);
> + if (err)
> + goto out_unlock_up;
> + }
>
> err = -EPERM;
> if (current->euid != ipcp->cuid &&
> @@ -468,10 +473,6 @@ asmlinkage long sys_msgctl (int msqid, i
> switch (cmd) {
> case IPC_SET:
> {
> - err = audit_ipc_set_perm(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode, ipcp);
> - if (err)
> - goto out_unlock_up;
> -
> err = -EPERM;
> if (setbuf.qbytes > msg_ctlmnb && !capable(CAP_SYS_RESOURCE))
> goto out_unlock_up;
> --- linux-2.6.16.x86_64.orig/ipc/shm.c 2006-05-05 15:08:23.000000000 -0400
> +++ linux-2.6.16.x86_64/ipc/shm.c 2006-05-05 14:51:53.000000000 -0400
> @@ -643,7 +643,7 @@ asmlinkage long sys_shmctl (int shmid, i
> 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));
> + err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode);
> if (err)
> goto out_unlock_up;
> err=-EPERM;
> --- linux-2.6.16.x86_64.orig/ipc/sem.c 2006-05-05 14:30:02.000000000 -0400
> +++ linux-2.6.16.x86_64/ipc/sem.c 2006-05-05 14:50:58.000000000 -0400
> @@ -828,6 +828,11 @@ static int semctl_down(int semid, int se
> if (err)
> goto out_unlock;
>
> + if (cmd == IPC_SET) {
> + err = audit_ipc_set_perm(0, setbuf.uid, setbuf.gid, setbuf.mode);
> + if (err)
> + goto out_unlock;
> + }
> if (current->euid != ipcp->cuid &&
> current->euid != ipcp->uid && !capable(CAP_SYS_ADMIN)) {
> err=-EPERM;
> @@ -844,9 +849,6 @@ static int semctl_down(int semid, int se
> err = 0;
> break;
> case IPC_SET:
> - 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;
> ipcp->mode = (ipcp->mode & ~S_IRWXUGO)
> --- linux-2.6.16.x86_64.orig/include/linux/audit.h 2006-05-05 15:09:42.000000000 -0400
> +++ linux-2.6.16.x86_64/include/linux/audit.h 2006-05-05 14:49:35.000000000 -0400
> @@ -324,7 +324,7 @@ extern void auditsc_get_stamp(struct aud
> 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_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_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode);
> extern int audit_bprm(struct linux_binprm *bprm);
> extern int audit_socketcall(int nargs, unsigned long *args);
> extern int audit_sockaddr(int len, void *addr);
>
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>
More information about the Linux-audit
mailing list