Re: [PATCH] change lspp inode auditing

> 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.

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.

> @@ -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?

> +             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.

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.