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 v3 2/2] s390/vfio-ap: control access to PQAP(AQIC)
 interception handler
To:     Jason Gunthorpe <jgg@nvidia.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,
        alex.williamson@redhat.com, kwankhede@nvidia.com
References: <20210519153921.804887-1-akrowiak@linux.ibm.com>
 <20210519153921.804887-3-akrowiak@linux.ibm.com>
 <20210519161610.GO1002214@nvidia.com>
From:   Tony Krowiak <akrowiak@linux.ibm.com>
Message-ID: <8c93c29a-e223-ac9a-5b54-7329587084c9@linux.ibm.com>
Date:   Wed, 19 May 2021 19:04:46 -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: <20210519161610.GO1002214@nvidia.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


On 5/19/21 12:16 PM, Jason Gunthorpe wrote:
> On Wed, May 19, 2021 at 11:39:21AM -0400, Tony Krowiak wrote:
>
>> @@ -287,13 +289,17 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
>>   	if (!(vcpu->arch.sie_block->eca & ECA_AIV))
>>   		return -EOPNOTSUPP;
>>   
>> -	apqn = vcpu->run->s.regs.gprs[0] & 0xffff;
>> -	mutex_lock(&matrix_dev->lock);
>> +	rcu_read_lock();
>> +	pqap_module_hook = rcu_dereference(vcpu->kvm->arch.crypto.pqap_hook);
>> +	if (!pqap_module_hook) {
>> +		rcu_read_unlock();
>> +		goto set_status;
>> +	}
>>   
>> -	if (!vcpu->kvm->arch.crypto.pqap_hook)
>> -		goto out_unlock;
>> -	matrix_mdev = container_of(vcpu->kvm->arch.crypto.pqap_hook,
>> -				   struct ap_matrix_mdev, pqap_hook);
>> +	matrix_mdev = pqap_module_hook->data;
>> +	rcu_read_unlock();
>> +	mutex_lock(&matrix_dev->lock);
> The matrix_mdev pointer was extracted from the pqap_module_hook,
> but now there is nothing protecting it since the rcu was dropped and
> it gets freed in vfio_ap_mdev_remove.

Therein lies the rub. We can't hold the rcu_read_lock across the
entire time that the interception is being processed because of
wait conditions in the interception handler. Regardless of whether
the pointer to the matrix_mdev is retrieved as the container of
or extracted from the pqap_hook, there is nothing protecting it
and there appears to be no way to do so using RCU.
>
> And, again, module locking doesn't prevent vfio_ap_mdev_remove() from
> being called. None of these patches should be combining module locking
> with RCU.

Is there any other way besides user interaction with the mdev's
sysfs remove interface for the remove callback to get invoked?
If I try to remove the mdev using the sysfs interface while the
mdev fd is still open by the guest, the remove hangs until the
fd is closed. That being the case, the mdev release callback
will get invoked prior to the remove callback being invoked which
renders this whole debate moot. What am I missing here?

>
> Jason