[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] change lspp inode auditing



On Wednesday 29 March 2006 14:01, Stephen Smalley wrote:
>> This patch brings the performance hit from 146% down to 11%. We need a
>> similar patch for IPC syscall auditing. 
>
> Not that I disagree with this change in approach, but I think that when
> it has come up in the past, there has been concern expressed about the
> fact that we could end up not being able to generate the context from
> the SID when the audit record is being emitted (due to OOM condition),
> and the operation has already occurred at that point.

In that case, the patch writes out the sid number. Given a sid, is there a way 
to find it in the policy on disk? If not, that might be useful to have.

> Of course, there are also other potential failure cases at the point, so I'm
> not sure it is crucial, as long as audit_panic is called as
> appropriate. 

If we record the sid number, do we really need to call audit_panic?

> > @@ -76,6 +78,26 @@ void selinux_audit_set_callback(int (*ca
> >   */
> >  void selinux_task_ctxid(struct task_struct *tsk, u32 *ctxid);
> >  
> > +/**
> > + *     selinux_ctxid_to_string - map a security context ID to a string
> > + *     @ctxid: security context ID to be converted.
> > + *     @ctx: address of context string to be returned
> > + *     @ctxlen: length of returned context string.
> > + *
> > + *     Returns 0 if successful, -errno if not.  On success, the context
> > + *     string will be allocated internally, and the caller must call
> > + *     kfree() on it after use.
> > + */
> > +int selinux_ctxid_to_string(u32 ctxid, char **ctx, u32 *ctxlen);
>
> Didn't Tim's patch for saving and auditing the netlink sender
> SID/context have a similar interface, based on James' proposed API for
> iptables?

Yes, I copy and pasted and changed the name based on a suggestion from Darrel. 
What is the status of that API? Did it go into 2.6.17 tree? I'd like to code 
to that API if it were available.

> > +             if (context->names[i].osid != 0) {
> > +                     char *ctx = NULL;
> > +                     int len = 0;
> > +                     if (selinux_ctxid_to_string(
> > +                             context->names[i].osid, &ctx, &len) == 0) {
> > +                             ctx = kmalloc(len, gfp_mask);
> > +                             if (ctx) {
> > +                                     selinux_ctxid_to_string(
> > +                                             context->names[i].osid,
> > +                                             &ctx, &len);
> > +                             }
> > +                     }
>
> Unless I'm confused (quite possible ;), the above sequence shouldn't be
> necessary and will actually leak the allocated buffer because SELinux
> will overwrite the pointer with its own.

OK, will look into this.

> Some of the hook interfaces unfortunately require the caller to guess and
> provide a buffer that they allocate, but I don't think we want to continue
> that trend.

Agreed, that was messy.

I'll make changes as you suggested and we can try this again. Is there a place 
I can grab James' iptables SE Linux interface to patch the lspp kernel with? 
I'd like to use that if its accepted/done. It'll make merging Tim's patch 
easier.

-Steve


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]