Re: Another slab size-32 leak 2.6.16-rc4-mm2
On Mon, 2006-02-27 at 12:31 -0500, Amy Griffis wrote:
You are correct that the code as written isn't doing anything
other
than leaking memory.
Unfortunately, that's right. The call to audit_ipc_context(ipcp) in
ipc/util.c returns a kmalloc'ed char * that isn't stored anywhere and
quite clearly leaks memory.
I set off on a search through the mailing list posts and my locally
stored patch revisions to determine how something like that slipped
through.
See the post:
https://www.redhat.com/archives/linux-audit/2005-October/msg00051.html
On Wed, 19 Oct 2005 16:39:21 -0500, I wrote:
- This patch reduced some of the verbage required for several
functions
and variables. In several places, "security_context" was replaced with
"context", such as
s/audit_ipc_security_context/audit_ipc_context/g
s/audit_log_task_security_context/audit_log_task_context/g
s/audit_aux_data_security_context/audit_aux_data_context/g
s/audit_inode_security_context/audit_inode_context/g
Additionally, the audit_names data structure uses char *ctx instead of
char *security_context now (which fits in more with gid,rdev,flags,
etc).
Please let me know if anyone is offended by these unrequested changes.
If this seems good enough to start testing, I'd like to see David merge
into his try by the end of the week. Thanks!
A couple of things happened that evidently flew under the radar with no
one screaming until now.
1) audit_ipc_perms() got struct kern_ipc_perm *ipcp added as the last
parameter
2) audit_ipc_security_context() was renamed to audit_ipc_context()
3) audit_ipc_context() was changed to return a char * of the context
string, which is called by audit_ipc_perms() (and helped clean up some
of the code in that function)
At this point, I neglected to kill the call to audit_ipc_context() in
ipc/util.c. Since each call to audit_ipc_perms() was now also passing
struct kern_ipc_perm *ipcp, the call in ipc/util.c was not needed.
As such, I ack Steve's patch. It is appropriate to delete the call to
audit_ipc_context() and furthermore make audit_ipc_context() local to
kernel/auditsc.c.
However, it was intended to collect labels for
message queues during calls to msgget(), msgrcv(), msgsnd(), etc. The
audit_ipc_perms() hook is only collecting labels (and attempted perm
settings) from IPC_SET operations.
I talked to Klaus about this and I expect him to pipe in right here...
In a nutshell, I was advised back in October that for certification
purposes, we're only required to audit ipc operations involving
security-relevant permissions checks (similar to our certification
requirements on syscall auditing). This is Klaus's area, though, so
I'll defer to his expertise here.
This call shouldn't be removed, it should be storing the
retrieved
label in the appropriate place in the audit context. The label would
then be freed in audit_free_aux().
This should be happening in the calls in:
ipc/msg.c, ipc/sem.c, ipc/shm.c:
audit_ipc_perms(setbuf.qbytes, setbuf.uid, setbuf.gid, setbuf.mode,
ipcp)
which calls audit_ipc_context() and gets the security context and stores
it in a (later freed) auxiliary context.
:-Dustin