Re: KASAN: use-after-free Read in ccid_hc_tx_delete

[Dan Carpenter]

On Thu, Nov 21, 2019 at 11:14:33PM +0300, Dan Carpenter wrote:
> On Thu, Nov 21, 2019 at 07:00:00AM -0800, syzbot wrote:
> > syzbot has bisected this bug to:
> > 
> > commit f04684b4d85d6371126f476d3268ebf6a0bd57cf
> > Author: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > Date:   Thu Jun 21 08:07:21 2018 +0000
> > 
> >     ALSA: lx6464es: Missing error code in snd_lx6464es_create()
> > 
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=10dd11cae00000
> > start commit:   eb6cf9f8 Merge tag 'arm64-fixes' of git://git.kernel.org/p..
> > git tree:       upstream
> > final crash:    https://syzkaller.appspot.com/x/report.txt?x=12dd11cae00000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=14dd11cae00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=c8970c89a0efbb23
> > dashboard link: https://syzkaller.appspot.com/bug?extid=3967c1caf256f4d5aefe
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11022ccd400000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=124581db400000
> > 
> > Reported-by: syzbot+3967c1caf256f4d5aefe@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: f04684b4d85d ("ALSA: lx6464es: Missing error code in
> > snd_lx6464es_create()")
> > 
> > For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> 
> This crash isn't related to my commit, it's seems something specific to
> DCCP.
> 
> My guess is that the fix is probably something like this.  The old sk
> and the new sk re-use the same newdp->dccps_hc_rx/tx_ccid pointers.
> The first sk destructor frees it and that causes a use after free when
> the second destructor tries to free it.
> 
> But I don't know DCCP code at all so I might be totally off and I
> haven't tested this at all...  It was just easier to write a patch than
> to try to explain in words.  Maybe we should clone the ccid instead of
> setting it to NULL.  Or I might be completely wrong.
> 
> ---
>  net/dccp/minisocks.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
> index 25187528c308..4cbfcccbbbbb 100644
> --- a/net/dccp/minisocks.c
> +++ b/net/dccp/minisocks.c
> @@ -98,6 +98,8 @@ struct sock *dccp_create_openreq_child(const struct sock *sk,
>  		newdp->dccps_timestamp_echo = dreq->dreq_timestamp_echo;
>  		newdp->dccps_timestamp_time = dreq->dreq_timestamp_time;
>  		newicsk->icsk_rto	    = DCCP_TIMEOUT_INIT;
> +		newdp->dccps_hc_rx_ccid     = NULL;
> +		newdp->dccps_hc_tx_ccid     = NULL;
>  
>  		INIT_LIST_HEAD(&newdp->dccps_featneg);
>  		/*

Could someone take a look at this?  It seem like a pretty serious bug
but DCCP is not very actively maintained and a lot of distributions
disable it.

regards,
dan carpenter