[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