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

Re: Another slab size-32 leak 2.6.16-rc4-mm2



On Wed, 2006-03-01 at 13:52 -0500, Amy Griffis wrote:
> Please take a closer look at the code.  The function that is
> collecting the ipc object label -- audit_ipc_context() -- is called in
> two places: audit_ipc_perms() and ipcperms().
> 
> audit_ipc_perms() is invoked during the following operations:
> 
>     msgctl - IPC_SET
>     semctl - IPC_SET
>     shmctl - IPC_SET
> 
> ipcperms() is invoked during the following operations:
> 
>     msgctl - IPC_STAT
>     msgsnd
>     msgrcv
>     semget
>     semctl - SEM_STAT
>     semctl - SETALL
>     semtimedop
>     shmget
>     shmctl - IPC_STAT
>     shmat
> 
> If you remove the audit_ipc_context() call from ipcperms() you will
> not be collecting object labels for the second set of operations.
> This does not meet LSPP requirements.
> 
> Your patch claims to collect object labels for ipc operations.  But
> since it only attaches the label to the audit context for the IPC_SET
> calls, it does not do what it claims.  At a minimum, your patch needs
> to be fixed to attach the object label to the audit context for the
> second set of operations.

Thank you, this is the analysis that I was looking for.

I'm in-lining a simple patch that solves memory leak and collects the
required information.  Rather than calling audit_ipc_context() which
allocates memory and returns a char * which was being lost, ipcperms()
instead calls audit_ipc_perms(), which wraps audit_ipc_context() thereby
storing the context in an auxiliary IPC audit record.  This happens each
and every time ipcperms() is called.

Note that the first argument of audit_ipc_perms() is qbytes, which I've
zero'd out inside of ipcperms() as that information is not available
within the scope of the ipcperms() function.  Is that ok?

One more note...  I'm attaching a short blip of test code that runs
through each of the msg* calls Amy mentions above.  The code doesn't do
anything useful with IPC msg's, but I used it to verify that audit
messages are generated and subj/obj labels are collected.

Thanks.

:-Dustin

diff --git a/ipc/util.c b/ipc/util.c
index e37e1e9..07fff36 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -469,7 +469,7 @@ int ipcperms (struct kern_ipc_perm *ipcp
 {	/* flag will most probably be 0 or S_...UGO from <linux/stat.h> */
 	int requested_mode, granted_mode;
 
-	audit_ipc_context(ipcp);
+	audit_ipc_perms(0, ipcp->uid, ipcp->gid, ipcp->mode, ipcp);
 	requested_mode = (flag >> 6) | (flag >> 3) | flag;
 	granted_mode = ipcp->mode;
 	if (current->euid == ipcp->cuid || current->euid == ipcp->uid)




#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/msg.h>
#include <sys/errno.h>
#include <sys/stat.h>
#include <stdio.h>

int main() {

	int rc = 0;
	int msgid = 0;
	int mode;
	struct msqid_ds buf;
	struct msgbuf {
		long mtype;
		char mtext[1];
	} mbuf;

	buf.msg_perm.uid = 0;
	buf.msg_perm.gid = 0;
	buf.msg_perm.mode = 0;

	mbuf.mtype = 1;

	mode = S_IRWXU;
	if( ( msgid = msgget( IPC_PRIVATE, mode ) ) == -1 ) {
		printf("ERROR: Unable to allocate new message queue: errno=%i\n", errno );
		goto EXIT;
	}

	if( ( rc = msgctl( msgid, IPC_SET, &buf ) ) == -1 ) {
		printf("ERROR: Unable to IPC_SET queue: errno=%i\n", errno );
		goto EXIT;
	}

	if( ( rc = msgctl( msgid, IPC_STAT, &buf ) ) == -1 ) {
		printf("ERROR: Unable to IPC_STAT queue: errno=%i\n", errno );
		goto EXIT;
	}

	if( ( rc = msgsnd( msgid, &mbuf, 0, 0 ) ) == -1 ) {
		printf("ERROR: Unable to send message: errno=%i\n", errno );
		goto EXIT;
	}

	if( ( rc = msgrcv( msgid, &mbuf, 0, 1, 0 ) ) == -1 ) {
		printf("ERROR: Unable to receive message: errno=%i\n", errno );
		goto EXIT;
	}

	if( ( rc = msgctl( msgid, IPC_RMID, 0 ) ) == -1 ) {
		printf( "Error removing message queue with ID = [%d]: errno = [%i]\n", msgid, errno );
		goto EXIT;
	}

EXIT:
	return rc;
}


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