Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 24 Apr 2020 15:22:23 -0300
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Divya Indi <divya.indi@oracle.com>
Cc:     linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
        =?utf-8?B?SMOla29u?= Bugge <haakon.bugge@oracle.com>,
        Kaike Wan <kaike.wan@intel.com>,
        Doug Ledford <dledford@redhat.com>,
        Gerd Rausch <gerd.rausch@oracle.com>,
        Srinivas Eeda <srinivas.eeda@oracle.com>,
        Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
Subject: Re: Request for feedback : Possible use-after-free in routing SA
 query via netlink
Message-ID: <20200424182223.GI26002@ziepe.ca>
References: <8fbdf10e-3f08-6407-eb0d-a1bf663873c3@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8fbdf10e-3f08-6407-eb0d-a1bf663873c3@oracle.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Apr 24, 2020 at 08:28:09AM -0700, Divya Indi wrote:

> If we look at the query, it does not appear to be a valid ib_sa_query. Instead
> looks like a pid struct for a process -> Use-after-free situation.
> 
> We could simulate the crash by explicitly introducing a delay in ib_nl_snd_msg with
> a sleep. The timer kicks in before ib_nl_send_msg has even sent out the request 
> and releases the query. We could reproduce the crash with a similar stack trace.
> 
> To summarize - We have a use-after-free possibility here when the timer(ib_nl_request_timeout)
> kicks in before ib_nl_snd_msg has completed sending the query out to ibacm via netlink. The 
> timeout handler ie ib_nl_request_timeout may result in releasing the query while ib_nl_snd_msg 
> is still accessing query.
> 
> Appreciate your thoughts on the above issue.

Your assesment looks right to me.

Fixing it will require being able to clearly explain what the lifetime
cycle is for ib_sa_query - and what is there today looks like a mess,
so I'm not sure what it should be changed into.

There is lots of other stuff there that looks really weird, like
ib_sa_cancel_query() keeps going even though there are still timers
running..

Jason