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

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>] [day] [month] [year] [list]

Message-ID: <20191029184855.GH6128@ziepe.ca>
Date:   Tue, 29 Oct 2019 15:48:55 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Hillf Danton <hdanton@...a.com>
Cc:     syzbot <syzbot+57a3b121df74c4eccbc7@...kaller.appspotmail.com>,
        bvanassche@....org, danitg@...lanox.com, dledford@...hat.com,
        leon@...nel.org, linux-kernel@...r.kernel.org,
        linux-rdma@...r.kernel.org, mhjungk@...il.com, parav@...lanox.com,
        shamir.rabinovitch@...cle.com, swise@...ngridcomputing.com,
        syzkaller-bugs@...glegroups.com, willy@...radead.org
Subject: Re: KASAN: use-after-free Read in cma_cancel_listens

On Thu, Oct 24, 2019 at 07:57:00PM +0800, Hillf Danton wrote:
> Detect and avoid repeated cancelation.
> 
> +++ b/drivers/infiniband/core/cma.c
> @@ -1747,7 +1747,9 @@ static void cma_cancel_listens(struct rd
>  	 * additional listen requests.
>  	 */
>  	mutex_lock(&lock);
> -	list_del(&id_priv->list);
> +	if (list_empty(&id_priv->list))
> +		goto unlock;
> +	list_del_init(&id_priv->list);
>  
>  	while (!list_empty(&id_priv->listen_list)) {
>  		dev_id_priv = list_entry(id_priv->listen_list.next,
> @@ -1760,6 +1762,7 @@ static void cma_cancel_listens(struct rd
>  		rdma_destroy_id(&dev_id_priv->id);
>  		mutex_lock(&lock);
>  	}
> +unlock:
>  	mutex_unlock(&lock);
>  }

Hum, it seems like a harmless change, but the real issue here is that
cma_cancel_listens() was called twice at all.

It seems pretty clear that the intent was it would be called on the
state, and the state is transitioned away before it is called. Ie see
how cma_cancel_operation() works with the 'state' argument.

So the only way to trigger this is to race two state transitions,
which means this is the usual syzkaller bug, the 'cma_exch'
synchronization scheme is just totally broken.

Jason