linux-kernel - Re: KASAN: use-after-free Read in ucma

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200914150149.GF1221970@ziepe.ca>
Date:   Mon, 14 Sep 2020 12:01:49 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Hillf Danton <hdanton@...a.com>
Cc:     syzbot <syzbot+cc6fc752b3819e082d0c@...kaller.appspotmail.com>,
        dledford@...hat.com, leon@...nel.org, linux-kernel@...r.kernel.org,
        linux-rdma@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: KASAN: use-after-free Read in ucma_close (2)

On Sat, Sep 12, 2020 at 09:35:14AM +0800, Hillf Danton wrote:
> > Migrate is the cause, very tricky:
> > 
> > 		CPU0                      CPU1
> > 	ucma_destroy_id()
> > 				  ucma_migrate_id()
> > 				       ucma_get_ctx()
> > 	xa_lock()
> > 	 _ucma_find_context()
> > 	 xa_erase()
> > 				       xa_lock()
> > 					ctx->file = new_file
> > 					list_move()
> > 				       xa_unlock()
> > 				      ucma_put_ctx
> > 				   ucma_close()
> > 				      _destroy_id()
> > 
> > 	_destroy_id()
> > 	  wait_for_completion()
> > 	  // boom
> > 
> > 
> > ie the destrory_id() on the initial FD captures the ctx right before
> > migrate moves it, then the new FD closes calling destroy while the
> > other destroy is still running.
> 
> More trouble now understanding that the ctx is reported to be freed
> in the write path, while if I dont misread the chart above, you're
> trying to pull another closer after migrate into the race.

migrate moves the ctx between two struct file's, so the race is to be
destroying on fir the first struct file, move to the second struct
file, then close the second struct file. 

Now close and destroy_id can race directly, which shouldn't be
allowed.

Jason