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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 07 Sep 2011 23:26:06 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Tim Chen <tim.c.chen@...ux.intel.com>
Cc:	"Yan, Zheng" <zheng.z.yan@...el.com>,
	"Yan, Zheng" <yanzheng@...n.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
	"jirislaby@...il.com" <jirislaby@...il.com>,
	"sedat.dilek@...il.com" <sedat.dilek@...il.com>,
	"Shi, Alex" <alex.shi@...el.com>,
	Valdis Kletnieks <Valdis.Kletnieks@...edu>
Subject: Re: [PATCH -next v2] unix stream: Fix use-after-free crashes

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@...ux.intel.com>
> Reported-by: Jiri Slaby <jirislaby@...il.com>
> Tested-by: Sedat Dilek <sedat.dilek@...glemail.com>
> Tested-by: Valdis Kletnieks <Valdis.Kletnieks@...edu>
> ---

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

Thanks !


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html