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;
Subject: Re: Request for feedback : Possible use-after-free in routing SA
 query via netlink
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
        =?UTF-8?Q?H=c3=a5kon_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>
References: <8fbdf10e-3f08-6407-eb0d-a1bf663873c3@oracle.com>
 <20200424182223.GI26002@ziepe.ca>
From:   Divya Indi <divya.indi@oracle.com>
Message-ID: <0ca6475a-4f07-44e1-1669-9dd815b521ae@oracle.com>
Date:   Thu, 30 Apr 2020 08:18:41 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0)
 Gecko/20100101 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200424182223.GI26002@ziepe.ca>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jason,

Thanks for taking the time to review. 

On 4/24/20 11:22 AM, Jason Gunthorpe wrote:
> 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.

The issue reported here appears to be restricted to the ibacm code path. 
Hence, we tried to resolve it for the life cycle of sa_query in the ibacm code path.
I will follow up this email with a proposed fix(patch), it would be very helpful if
you can provide your feedback on the same.

Thanks,
Divya

>
> The
> re 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