linux-kernel - Re: [PATCH v3 1/9] s390: vfio_ap: link the vfio_ap devices to the vfio

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <f95dec76-ba2e-38b2-64f3-9abe4ca6a501@linux.ibm.com>
Date:   Tue, 19 Feb 2019 17:27:05 -0500
From:   Tony Krowiak <akrowiak@...ux.ibm.com>
To:     Cornelia Huck <cohuck@...hat.com>
Cc:     pmorel@...ux.ibm.com, borntraeger@...ibm.com,
        alex.williamson@...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 1/9] s390: vfio_ap: link the vfio_ap devices to the
 vfio_ap bus subsystem

On 2/18/19 11:57 AM, Cornelia Huck wrote:
> On Mon, 18 Feb 2019 11:35:45 -0500
> Tony Krowiak <akrowiak@...ux.ibm.com> wrote:
> 
>> On 2/18/19 7:01 AM, Cornelia Huck wrote:
>>> On Fri, 15 Feb 2019 16:59:33 -0500
>>> Tony Krowiak <akrowiak@...ux.ibm.com> wrote:
>>>    
>>>> On 2/15/19 4:11 AM, Cornelia Huck wrote:
>>>>> On Thu, 14 Feb 2019 13:30:59 -0500
>>>>> Tony Krowiak <akrowiak@...ux.ibm.com> wrote:
>>>>>       
>>>>>> On 2/14/19 12:36 PM, Pierre Morel wrote:
>>>>>>> On 14/02/2019 17:57, Cornelia Huck wrote:
>>>    
>>>>>>>> (And reading further in the current code, it seems we clear that
>>>>>>>> structure _after_ the matrix device had been setup, so how can that
>>>>>>>> even work? Where am I confused?)
>>>>>>>
>>>>>>> On device_register there were no bus, so the core just do not look for a
>>>>>>> driver and this field was nor tested nor overwritten.
>>>>>
>>>>> Hm... so has the callback in driver_for_each_device() in
>>>>> vfio_ap_verify_queue_reserved() ever been invoked at all? It seems this
>>>>> patch fixes more than just libudev issues...
>>>>
>>>> It is this patch that rendered the driver_for_each_device() in
>>>> vfio_ap_verify_queue_reserved() erroneous. That function gets called
>>>> every time an adapter or domain is assigned to the mdev. This patch
>>>> introduced the problem with driver_for_each_device().
>>>
>>> So, does this function need to be removed or called from another place,
>>> then? (It looks like it was dead code before.)
>>
>> I don't see why you think it's dead code:
>>
>> assign_adapter_store
>> ==> vfio_ap_mdev_verify_queues_reserved_for_apid
>>       ==> vfio_ap_verify_queue_reserved
>>           ==> driver_for_each_device
>>
>> The only way that the vfio_ap_verify_queue_reserved - the function that
>> calls driver_for_each_device - does not get called is if no bits have
>> yet been set in matrix_mdev->matrix.aqm.
> 
> What I don't see is how this can be called if no device has been, in
> fact, bound to the driver in the driver core...

Let's start with the fact that one can create an mdev device regardless
of whether a queue has been bound to the vfio_ap driver. Once an mdev
device is created, one can start assigning adapters, domains and control
domains to it. Let's say the admin now attempts to assign an adapter, in
which case the assign_adapter_store() function is invoked. After
verifying that the APID passed in is a valid adapter number, the
vfio_ap_mdev_verify_queues_reserved_for_apid() function is called.
This function first checks if any domains have been assigned and if not,
calls vfio_ap_verify_queue_reserved(&apid, NULL). It is in this function
that the driver_for_each_device() function is called. Since there are
no devices bound to the vfio_ap device driver, the callback passed in to
the driver_for_each_device() function will never get called, so the
vfio_ap_mdev_verify_queues_reserved_for_apid() function will return
-EADDRNOTAVAIL. A similar flow will occur if the first assignment is for
a domain. The bottom line is, the driver_for_each_device() function is
called every time an adapter or domain is assigned.

>