[PATCH] c/r: Initialize msg_msg security pointer at restart.

Serge Hallyn serge.hallyn at canonical.com
Thu Mar 31 06:08:29 PDT 2011


Quoting Jose R. Santos (jrs at linux.vnet.ibm.com):
> On Wed, 30 Mar 2011 22:29:54 -0500
> "Serge E. Hallyn" <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Jose R. Santos (jrs at linux.vnet.ibm.com):
> > > This small oversight was causing either crashes on free_msg() or
> > > restart failures under some message queue workloads.
> > > 
> > > Signed-off-by: Jose R. Santos <jrs at linux.vnet.ibm.com>
> > 
> > What kernel is this?
> 
> ckpt-v23-rc1
> 
> > Starting with the last line in your context, there is:
> > 
> > 	/* set default MAC attributes */
> > 	ret = security_msg_msg_alloc(msg);
> > 	if (ret < 0)
> > 		goto out;
> > 
> > which should take care of that.  I don't know of an LSM which
> > doesn't define msg_msg_alloc() but does define msg_msg_free().
> > Do you have a stack trace to show where it was getting
> > freed?
> 
> Sorry, should have put more details in the patch description.  The
> problem here is that msg_msg_alloc() in not allocating anything and
> thus the msg->security just happens to have what ever random data
> that happens to be there after kmalloc was called for this msg. So the
> way this usually manifested was during the a second checkpoint/restart.
> For example:
> 
> run ./mq_test
> checkpoint:
> 	Saving the msg with SECURITY_NONE
> restart:
> 	Restores the msg content but with msg->security set to
> 	something random (or 0x5a pattern if using DEBUG_SLAB)
> second checkpoint:
> 	msg->security not NULL so we proceed in
> 	security_checkpoint_obj() to call security_msg_msg_checkpoint()
> second restart:
> 	All sorts of very weird, very different, hard to debug stuff
> 	happens afterwards. :)
> 
> Also, If you look at load_msg() in msgutil.c, the msg->security is also
> initialized to NULL _before_ calling security_msg_msg_alloc().  Hope
> this explains this fix better.
> 
> -JRS

I see - thanks for the explanation.

Part of me would prefer to see the security hooks make sure that they
set it, but that might thwart the people currently playing with LSM
stacking, so your way is probably best.

Acked-by: Serge Hallyn <serge.hallyn at ubuntu.com>

thanks,
-serge


More information about the Containers mailing list