Re: [PATCH -next v2] unix stream: Fix use-after-free crashes

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mercredi 07 septembre 2011 à 05:01 -0700, Tim Chen a écrit :
> On Wed, 2011-09-07 at 09:45 +0200, Eric Dumazet wrote:
> > Le mercredi 07 septembre 2011 à 13:20 +0800, Yan, Zheng a écrit :
> > 
> > > Is code like this OK? Thanks
> > > ---
> > > 	if (sent + size < len) { 
> > > 		/* Only send the fds in the first buffer */
> > > 		/* get additional ref if more skbs will be created */
> > > 		err = unix_scm_to_skb(siocb->scm, skb, !fds_sent, true);
> > > 	} else {
> > > 		err = unix_scm_to_skb(siocb->scm, skb, !fds_sent, false);
> > > 		ref_avail = false;
> > > 	}
> > > 
> > > 
> > 
> > Whats wrong with using ref_avail in the unix_scm_to_skb() call itself ?
> > 
> > something like :
> > 
> 
> Eric,
> 
> Your updated patch looks good when I tested it on my side.  It makes the
> patch much more readable.  If this patch looks good with you and Yan
> Zheng, can you and Yan Zheng add your Signed-off-by to the patch?
> 
> Jiri, Sedat or Valdis, if you can verify that the patch fixed commit
> 0856a30409, that will be appreciated.
> 
> Eric, are you planning to do a fast path patch that doesn't do pid ref
> for the case where CONFIG_PID_NS is not set?
> 

Yes, I'll try to cook a patch.

> Thanks.
> 
> Tim
> 
> ---
> 
> Commit 0856a30409 (Scm: Remove unnecessary pid & credential references
> in Unix socket's send and receive path) introduced a use-after-free bug.
> The sent skbs from unix_stream_sendmsg could be consumed and destructed 
> by the receive side, removing all references to the credentials, 
> before the send side has finished sending out all 
> packets. However, send side could continue to consturct new packets in the 
> stream, using credentials that have lost its last reference and been
> freed.  
> 
> In this fix, we don't steal the reference to credentials we have obtained 
> in scm_send at beginning of unix_stream_sendmsg, till we've reached
> the last packet.  This fixes the problem in commit 0856a30409.
> 
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> Reported-by: Jiri Slaby <jirislaby@gmail.com>
> Tested-by: Sedat Dilek <sedat.dilek@googlemail.com>
> Tested-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
> ---

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks !