Re: [PATCH] LSPP audit enablement: storing selinux ocontext and scontext
I've combed through the responses to my original patch. Collected below
are a distilled view of the direct criticisms I can address.
Updated patch attached. In-line comments below.
On Fri, 2005-07-22 at 13:46 -0400, David Woodhouse wrote:
On Thu, 2005-07-21 at 10:48 -0500, Dustin Kirkland wrote:
> I expect at least one section of this code to generate some animated
> discussion. This code utilizes "static inline" function definitions
> rather than "#define" macros, as per Documentation/SubmittingPatches
> lines 328-372 and Documentation/CodingStyle line 358. This is
> inconsistent with the rest of the code in include/linux/audit.h, so I
> suspect someone will have to give, in the interest of consistency.
> Please advise.
In this case the one-line #define is probably nicer. Let's stick with
that.
Ok, the static-inlines in linux/audit.h are now changed to #defines.
On Fri, 2005-07-22 at 13:46 -0400, David Woodhouse wrote:
Please use kstrdup() in audit_set_macxattr().
Done.
On Wed, 2005-07-27 at 12:17 -0400, Steve Grubb wrote:
On Monday 25 July 2005 14:28, Dustin Kirkland wrote:
> Bugs in the patch? I don't doubt you, I'm just curious... Can you
> cite?
I don't have lots of time right now. But, I think there should be some checks
for selinux_enabled.
Hmmm... Steve: Can you point me to some places where you feel this
might be necessary?
On Thu, 2005-07-28 at 10:49 -0400, Steve Grubb wrote:
Another issue...the patch is too eager to call audit_panic(). It is
more
correct to fail the syscall and let the app handle failure than bring the
machine to its knees.
On Thu, 2005-07-28 at 11:49 -0400, Steve Grubb wrote:
audit_panic is reserved for use when the backlog overflows or rate
limit is too high. When you wrote the fs watch code, did you call audit_panic
when a buffer alloc failed? No, you failed the syscall. The behavior should
be consistent.
Ok, so the audit_panic() code seems to still be under discussion. One
thing that's important to realize is that audit_panic() does not
necessarily panic the kernel. Depending on the value of audit_failure,
it can 1) fail silently, 2) fail with only a KERN_ERR printk, or 3) it
can panic the kernel. Let's come to a consensus on how these failures
should be handled...
So have another look at this patch and please make some constructive
suggestions if the audit_panic() situations should be handled
differently. I'd like to push this for inclusion in David's tree as
soon as possible.
Thanks,
:-Dustin
Attachments:
- signature.asc (application/pgp-signature — 189 bytes)
- 2.6.13-rc6-mm2-lspp_audit.patch (text/x-patch — 19.0 KB)