Re: [PATCH 3/15] sanitize audit_ipc_obj()

2008-12-17 Thread Eric Paris 
On Wed, 2008-12-17 at 20:53 +1100, James Morris wrote:
> On Wed, 17 Dec 2008, Al Viro wrote:
> 
> > On Wed, Dec 17, 2008 at 06:24:40PM +1100, James Morris wrote:
> > > On Wed, 17 Dec 2008, Al Viro wrote:
> > > 
> > > > +   struct {
> > > > +   uid_t   uid;
> > > > +   gid_t   gid;
> > > > +   mode_t  mode;
> > > > +   u32 osid;
> > > > +   } ipc;
> > > 
> > > 'osid' should be converted into 'secid' someday.
> > 
> > Eh?  Do you mean the field name there or the actual output?  Either is
> > trivial, of course, but the latter is up to userland folks and the
> > former alone seems to be rather pointless...
> 
> I was thinking in terms of the kernel API, where 'secid' is the preferred 
> name for security identifiers ('sid' being an SELinux-specific term and 
> also conflicting with 'session id').  Given that it's exposed to userland, 
> I guess it's too late.

James meant just do s/osid/secid/ for continuity across the kernel (we
are trying to make the main kernel a bit more LSM agnostic and sid is an
SELinux term).  The userspace exported part is actually a translated
string (I think we use ocontext= and scontext=).

There is no reason we couldn't do this in audit.  But, I don't think
it's worth changing this patch, as I think audit refers to it as sid in
other places.  Maybe I'll try to clean that up someday.  I at least
added it to my "someday" todo list.

-Eric

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH 3/15] sanitize audit_ipc_obj()

2008-12-17 Thread James Morris 
On Wed, 17 Dec 2008, Al Viro wrote:

> On Wed, Dec 17, 2008 at 06:24:40PM +1100, James Morris wrote:
> > On Wed, 17 Dec 2008, Al Viro wrote:
> > 
> > > + struct {
> > > + uid_t   uid;
> > > + gid_t   gid;
> > > + mode_t  mode;
> > > + u32 osid;
> > > + } ipc;
> > 
> > 'osid' should be converted into 'secid' someday.
> 
> Eh?  Do you mean the field name there or the actual output?  Either is
> trivial, of course, but the latter is up to userland folks and the
> former alone seems to be rather pointless...

I was thinking in terms of the kernel API, where 'secid' is the preferred 
name for security identifiers ('sid' being an SELinux-specific term and 
also conflicting with 'session id').  Given that it's exposed to userland, 
I guess it's too late.


- James
-- 
James Morris


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH 3/15] sanitize audit_ipc_obj()

2008-12-16 Thread James Morris 
On Wed, 17 Dec 2008, Al Viro wrote:

> + struct {
> + uid_t   uid;
> + gid_t   gid;
> + mode_t  mode;
> + u32 osid;
> + } ipc;

'osid' should be converted into 'secid' someday.


Reviewed-by: James Morris 

-- 
James Morris


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

[PATCH 3/15] sanitize audit_ipc_obj()

2008-12-16 Thread Al Viro 

* get rid of allocations
* make it return void
* simplify callers

Signed-off-by: Al Viro 
---
 include/linux/audit.h |9 ++---
 ipc/shm.c |4 +--
 ipc/util.c|9 ++---
 kernel/auditsc.c  |   88 
 4 files changed, 45 insertions(+), 65 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index e59feb9..97598f0 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -441,7 +441,7 @@ extern int  audit_set_loginuid(struct task_struct *task, 
uid_t loginuid);
 #define audit_get_loginuid(t) ((t)->loginuid)
 #define audit_get_sessionid(t) ((t)->sessionid)
 extern void audit_log_task_context(struct audit_buffer *ab);
-extern int __audit_ipc_obj(struct kern_ipc_perm *ipcp);
+extern void __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);
 extern int audit_bprm(struct linux_binprm *bprm);
 extern void audit_socketcall(int nargs, unsigned long *args);
@@ -454,11 +454,10 @@ extern int __audit_mq_timedreceive(mqd_t mqdes, size_t 
msg_len, unsigned int __u
 extern int __audit_mq_notify(mqd_t mqdes, const struct sigevent __user 
*u_notification);
 extern int __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat);
 
-static inline int audit_ipc_obj(struct kern_ipc_perm *ipcp)
+static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 {
if (unlikely(!audit_dummy_context()))
-   return __audit_ipc_obj(ipcp);
-   return 0;
+   __audit_ipc_obj(ipcp);
 }
 static inline int audit_fd_pair(int fd1, int fd2)
 {
@@ -522,7 +521,7 @@ extern int audit_signals;
 #define audit_get_loginuid(t) (-1)
 #define audit_get_sessionid(t) (-1)
 #define audit_log_task_context(b) do { ; } while (0)
-#define audit_ipc_obj(i) ({ 0; })
+#define audit_ipc_obj(i) ((void)0)
 #define audit_ipc_set_perm(q,u,g,m) ({ 0; })
 #define audit_bprm(p) ({ 0; })
 #define audit_socketcall(n,a) ((void)0)
diff --git a/ipc/shm.c b/ipc/shm.c
index 867e5d6..e40065e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -747,9 +747,7 @@ asmlinkage long sys_shmctl(int shmid, int cmd, struct 
shmid_ds __user *buf)
goto out;
}
 
-   err = audit_ipc_obj(&(shp->shm_perm));
-   if (err)
-   goto out_unlock;
+   audit_ipc_obj(&(shp->shm_perm));
 
if (!capable(CAP_IPC_LOCK)) {
err = -EPERM;
diff --git a/ipc/util.c b/ipc/util.c
index 361fd1c..0aa8ed8 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -620,10 +620,9 @@ 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, err;
+   int requested_mode, granted_mode;
 
-   if (unlikely((err = audit_ipc_obj(ipcp
-   return err;
+   audit_ipc_obj(ipcp);
requested_mode = (flag >> 6) | (flag >> 3) | flag;
granted_mode = ipcp->mode;
if (current->euid == ipcp->cuid || current->euid == ipcp->uid)
@@ -797,9 +796,7 @@ struct kern_ipc_perm *ipcctl_pre_down(struct ipc_ids *ids, 
int id, int cmd,
goto out_up;
}
 
-   err = audit_ipc_obj(ipcp);
-   if (err)
-   goto out_unlock;
+   audit_ipc_obj(ipcp);
 
if (cmd == IPC_SET) {
err = audit_ipc_set_perm(extra_perm, perm->uid,
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 1d53aa8..c136047 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -221,6 +221,12 @@ struct audit_context {
int nargs;
long args[6];
} socketcall;
+   struct {
+   uid_t   uid;
+   gid_t   gid;
+   mode_t  mode;
+   u32 osid;
+   } ipc;
};
 
 #if AUDIT_DEBUG
@@ -578,19 +584,12 @@ static int audit_filter_rules(struct task_struct *tsk,
}
}
/* Find ipc objects that match */
-   if (ctx) {
-   struct audit_aux_data *aux;
-   for (aux = ctx->aux; aux;
-aux = aux->next) {
-   if (aux->type == AUDIT_IPC) {
-   struct 
audit_aux_data_ipcctl *axi = (void *)aux;
-   if 
(security_audit_rule_match(axi->osid, f->type, f->op, f->lsm_rule, ctx)) {
-   ++result;
-   break;
-