linux-kernel - Re: [PATCH v3 8/9] s390: ap: Cleanup on removing the AP device

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Feb 2019 20:41:32 +0100
From:   Pierre Morel <pmorel@...ux.ibm.com>
To:     Tony Krowiak <akrowiak@...ux.ibm.com>, borntraeger@...ibm.com
Cc:     alex.williamson@...hat.com, cohuck@...hat.com,
        linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
        kvm@...r.kernel.org, frankja@...ux.ibm.com, pasic@...ux.ibm.com,
        david@...hat.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, freude@...ux.ibm.com, mimu@...ux.ibm.com
Subject: Re: [PATCH v3 8/9] s390: ap: Cleanup on removing the AP device

On 16/02/2019 00:36, Tony Krowiak wrote:
> On 2/14/19 8:51 AM, Pierre Morel wrote:
>> When the device is remove, we must make sure to
>> clear the interruption and reset the AP device.
>>
...snip...

>> @@ -74,6 +159,13 @@ static void vfio_ap_queue_dev_remove(struct 
>> ap_device *apdev)
>>       struct vfio_ap_queue *q;
>>       q = dev_get_drvdata(&apdev->device);
>> +    if (!q)
>> +        return;
>> +
>> +    vfio_ap_update_crycb(q);
> 
> The root user is warned in the Limitations section of the vfio-ap.txt
> doc delivered with the AP pass-through support warns that the
> administrator (i.e., root user) should ensure that AP devices are not
> removed without taking proper care to ensure they are not in use by a
> guest. I am currently working on a patch set to handle this, so this
> may simply get ripped out when those patches are integrated. That may
> very well be simultaneously with this patch series as I plan on posting
> those soon.
> 
> If this call is to remain, then you ought to update the vfio-ap.txt
> document to let users know that when queues are unbound, the guests
> will lose access to them unbeknown to the admin of the guest.

I do not see where is the problem, the admin should still take care the 
APQN are not in use by the guest when he does an unbind.

This just makes sure it is not used anymore by the guest when the admin 
rebound it to the host or another guest.

> 
>> +    vfio_ap_zapq(q);
> 
> One last thing. I've explained before that prior to the AP bus
> invoking this remove callback, it flushes and zeroizes the
> queue. Why do you insist it needs to be done again in the remove
> callback?

The problem is that the AP_BUS is not aware from the CRYCB and let the 
guest play with the queue.

The sequence an be like:

-> AP_BUS remove RESET the queue and zeroes with ZAPQ
-> AP_BUS call remove from driver
	- the APQN still belong to the guest !
		-> the guest issue a NQAP
===> We need to take the queue away from the guest
===> and we need to RESET the queue with ZAPQ and wait
      until no more message is in the queue
-> driver remove ends


Regards,
Pierre


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany