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: [PATCH v2] s390/vfio-ap: fix memory leak in mdev remove callback
To:     Halil Pasic <pasic@linux.ibm.com>
Cc:     linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        borntraeger@de.ibm.com, cohuck@redhat.com,
        pasic@linux.vnet.ibm.com, jjherne@linux.ibm.com, jgg@nvidia.com,
        alex.williamson@redhat.com, kwankhede@nvidia.com,
        stable@vger.kernel.org, Tony Krowiak <akrowiak@stny.rr.com>
References: <20210510214837.359717-1-akrowiak@linux.ibm.com>
 <20210512203536.4209c29c.pasic@linux.ibm.com>
 <4c156ab8-da49-4867-f29c-9712c2628d44@linux.ibm.com>
 <20210513194541.58d1628a.pasic@linux.ibm.com>
 <243086e2-08a0-71ed-eb7e-618a62b007e4@linux.ibm.com>
 <20210514021500.60ad2a22.pasic@linux.ibm.com>
 <594374f6-8cf6-4c22-0bac-3b224c55bbb6@linux.ibm.com>
 <20210517211030.368ca64b.pasic@linux.ibm.com>
From:   Tony Krowiak <akrowiak@linux.ibm.com>
Message-ID: <f9785365-0269-c22e-f9b1-c261a15dad23@linux.ibm.com>
Date:   Tue, 18 May 2021 09:41:35 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <20210517211030.368ca64b.pasic@linux.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


On 5/17/21 3:10 PM, Halil Pasic wrote:
> On Mon, 17 May 2021 09:37:42 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
>>> Because of this, I don't think the rest of your argument is valid.
>> Okay, so your concern is that between the point in time the
>> vcpu->kvm->arch.crypto.pqap_hook pointer is checked in
>> priv.c and the point in time the handle_pqap() function
>> in vfio_ap_ops.c is called, the memory allocated for the
>> matrix_mdev containing the struct kvm_s390_module_hook
>> may get freed, thus rendering the function pointer invalid.
>> While not impossible, that seems extremely unlikely to
>> happen. Can you articulate a scenario where that could
>> even occur?
> Malicious userspace. We tend to do the pqap aqic just once
> in the guest right after the queue is detected. I do agree
> it ain't very likely to happen during normal operation. But why are
> you asking?

I'm just trying to wrap my head around how this can
happen given the incredibly small window between
access to the pointer to the structure containing the
function pointer and access to the function pointer
itself.

>
> I'm not sure I understood correctly what kind of a scenario are
> you asking for. PQAP AQIC and mdev remove are independent
> events originated in userspace, so AFAIK we may not assume
> that the execution of two won't overlap, nor are we allowed
> to make assumptions on how does the execution of these two
> overlap (except for the things we explicitly ensure -- e.g.
> some parts are made mutually exclusive using the matrix_dev->lock
> lock).

It looks like we need a way to control access to the
struct kvm_s390_module_hook. I'm looking into
Christian's suggestion for using RCU as well as other
solutions.

>
> Regards,
> Halil
>