linux-kernel - Re: [PATCH V4 03/11] vfio/pci: Prepare for dynamic interrupt context storage

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] [day] [month] [year] [list]

Message-ID: <1f84c37d-585b-8113-b045-7d6076405e34@intel.com>
Date:   Mon, 8 May 2023 15:52:56 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "jgg@...dia.com" <jgg@...dia.com>,
        "yishaih@...dia.com" <yishaih@...dia.com>,
        "shameerali.kolothum.thodi@...wei.com" 
        <shameerali.kolothum.thodi@...wei.com>,
        "alex.williamson@...hat.com" <alex.williamson@...hat.com>
CC:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "darwi@...utronix.de" <darwi@...utronix.de>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        "Liu, Jing2" <jing2.liu@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        "tom.zanussi@...ux.intel.com" <tom.zanussi@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4 03/11] vfio/pci: Prepare for dynamic interrupt context
 storage

Hi Kevin,

On 5/5/2023 12:21 AM, Tian, Kevin wrote:
>> From: Chatre, Reinette <reinette.chatre@...el.com>
>> Sent: Saturday, April 29, 2023 2:24 AM
>>
>> Hi Kevin,
>>
>> On 4/27/2023 11:33 PM, Tian, Kevin wrote:
>>>> From: Chatre, Reinette <reinette.chatre@...el.com>
>>>> Sent: Friday, April 28, 2023 1:36 AM
>>>>
>>>> @@ -55,17 +80,28 @@ static void vfio_send_intx_eventfd(void *opaque,
>>>> void *unused)
>>>>  {
>>>>  	struct vfio_pci_core_device *vdev = opaque;
>>>>
>>>> -	if (likely(is_intx(vdev) && !vdev->virq_disabled))
>>>> -		eventfd_signal(vdev->ctx[0].trigger, 1);
>>>> +	if (likely(is_intx(vdev) && !vdev->virq_disabled)) {
>>>> +		struct vfio_pci_irq_ctx *ctx;
>>>> +
>>>> +		ctx = vfio_irq_ctx_get(vdev, 0);
>>>> +		if (!ctx)
>>>> +			return;
>>>
>>> if this error happens it implies a kernel bug since the same check
>>> has been done in vfio_intx_enable(). Then should be a WARN_ON().
>>
>> Sure. Considering that if these are triggered it may result
>> in many instances, so perhaps WARN_ON_ONCE()?
> 
> yes.
> 
>>
>>> ditto for other intx functions which can be called only after intx
>>> is enabled.
>>
>> It seems the instances in this category can be identified as the places
>> where the array contents is currently used without any checks.
>>
>> I am planning on the following changes:
>>
> 
> that looks good to me

Thank you so much for this guidance. After adding these WARNs two of them
were actually encountered and revealed that the interrupt context was retrieved
too early in vfio_pci_intx_mask() and vfio_pci_intx_unmask_handler() in
the scenario where these calls are triggered via config writes while INTx is
disabled. This will be fixed in the next version.

Reinette